From df10d5a17dd6c1062c9f25effbc499165bf6cb70 Mon Sep 17 00:00:00 2001 From: Devon Mizelle Date: Fri, 14 May 2021 16:35:40 -0400 Subject: [PATCH 1/2] Add includeCRDs Field to HelmChart This commit adds functionality for a user to specify that `helm` should include CRDs when inflating a Helm Chart. As of Helm v3, the `install-crd` hook is no more, with `CustomResourceDefinitions` existing in the root of the chart, under the `crds` directory. When calling `helm template`, `helm` does not output these CRDs unless the user passes the `--include-crds` flag to the command. With this commit, users can set `includeCRDs: true` as part of their helm chart definition in `kustomize.yaml` to have these included as part of the output. Signed-off-by: Devon Mizelle --- api/builtins/HelmChartInflationGenerator.go | 6 + api/types/helmchartargs.go | 4 + examples/chart.md | 5 + .../HelmChartInflationGenerator.go | 3 + .../HelmChartInflationGenerator_test.go | 90 ++++++ .../include_crds_testdata.txt | 262 ++++++++++++++++++ 6 files changed, 370 insertions(+) create mode 100644 plugin/builtin/helmchartinflationgenerator/include_crds_testdata.txt diff --git a/api/builtins/HelmChartInflationGenerator.go b/api/builtins/HelmChartInflationGenerator.go index 7037a01b5..2a654ad1f 100644 --- a/api/builtins/HelmChartInflationGenerator.go +++ b/api/builtins/HelmChartInflationGenerator.go @@ -267,6 +267,9 @@ func (p *HelmChartInflationGeneratorPlugin) templateCommand() []string { if p.ReleaseName != "" { args = append(args, p.ReleaseName) } + if p.Namespace != "" { + args = append(args, "--namespace", p.Namespace) + } args = append(args, filepath.Join(p.absChartHome(), p.Name)) if p.ValuesFile != "" { args = append(args, "--values", p.ValuesFile) @@ -277,6 +280,9 @@ func (p *HelmChartInflationGeneratorPlugin) templateCommand() []string { // I've tried placing the flag before and after the name argument. args = append(args, "--generate-name") } + if p.IncludeCRDs { + args = append(args, "--include-crds") + } return args } diff --git a/api/types/helmchartargs.go b/api/types/helmchartargs.go index 410601857..5f4fc622d 100644 --- a/api/types/helmchartargs.go +++ b/api/types/helmchartargs.go @@ -68,6 +68,10 @@ type HelmChart struct { // Legal values: 'merge', 'override', 'replace'. // Defaults to 'override'. ValuesMerge string `json:"valuesMerge,omitempty" yaml:"valuesMerge,omitempty"` + + // InstallCRDs specifies if Helm should also generate CustomResourceDefinitions. + // Defaults to 'false'. + IncludeCRDs bool `json:"includeCRDs,omitempty" yaml:"includeCRDs,omitempty"` } // HelmChartArgs contains arguments to helm. diff --git a/examples/chart.md b/examples/chart.md index 3f63485a1..fcaa94021 100644 --- a/examples/chart.md +++ b/examples/chart.md @@ -88,6 +88,7 @@ Define this base the usual way by creating a cat <<'EOF' >$DEMO_HOME/base/kustomization.yaml helmCharts: - name: minecraft + includeCRDs: false valuesInline: minecraftServer: eula: true @@ -105,6 +106,10 @@ field, specifying a single chart. The `valuesInline` field overrides some native chart values. +The `includeCRDs` field instructs Helm to generate +`CustomResourceDefinitions`. +See the [Helm docs for further info](https://helm.sh/docs/chart_best_practices/custom_resource_definitions/) + Check the directory layout: diff --git a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go index 84fbca807..4fd150262 100644 --- a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go +++ b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go @@ -285,6 +285,9 @@ func (p *HelmChartInflationGeneratorPlugin) templateCommand() []string { // I've tried placing the flag before and after the name argument. args = append(args, "--generate-name") } + if p.IncludeCRDs { + args = append(args, "--include-crds") + } return args } diff --git a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go index f187223e0..aeeb13164 100644 --- a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go +++ b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go @@ -2,6 +2,7 @@ package main_test import ( "fmt" + "io/ioutil" "path/filepath" "testing" @@ -458,3 +459,92 @@ valuesMerge: replace 111, // memory )) } + +func TestHelmChartInflationGeneratorWithIncludeCRDs(t *testing.T) { + th := kusttest_test.MakeEnhancedHarnessWithTmpRoot(t). + PrepBuiltin("HelmChartInflationGenerator") + defer th.Reset() + if err := th.ErrIfNoHelm(); err != nil { + t.Skip("skipping: " + err.Error()) + } + + // we store this data outside of the _test.go file as its sort of huge + // and has backticks, which makes string literals wonky + testData, err := ioutil.ReadFile("include_crds_testdata.txt") + if err != nil { + t.Errorf("unable to read test data for includeCRDs: %w", err) + } + + rm := th.LoadAndRunGenerator(` +apiVersion: builtin +kind: HelmChartInflationGenerator +metadata: + name: terraform +name: terraform +version: 1.0.0 +repo: https://helm.releases.hashicorp.com +releaseName: terraforming-mars +includeCRDs: true +valuesInline: + global: + enabled: false + tests: + enabled: false +`) + th.AssertActualEqualsExpected(rm, string(testData)) +} + +func TestHelmChartInflationGeneratorWithExcludeCRDs(t *testing.T) { + th := kusttest_test.MakeEnhancedHarnessWithTmpRoot(t). + PrepBuiltin("HelmChartInflationGenerator") + defer th.Reset() + if err := th.ErrIfNoHelm(); err != nil { + t.Skip("skipping: " + err.Error()) + } + + // we choose this helm chart as it has the ability to turn + // everything off, except CRDs. + rm := th.LoadAndRunGenerator(` +apiVersion: builtin +kind: HelmChartInflationGenerator +metadata: + name: terraform +name: terraform +version: 1.0.0 +repo: https://helm.releases.hashicorp.com +releaseName: terraforming-mars +includeCRDs: false +valuesInline: + global: + enabled: false + tests: + enabled: false +`) + th.AssertActualEqualsExpected(rm, "") +} + +func TestHelmChartInflationGeneratorWithIncludeCRDsNotSpecified(t *testing.T) { + th := kusttest_test.MakeEnhancedHarnessWithTmpRoot(t). + PrepBuiltin("HelmChartInflationGenerator") + defer th.Reset() + if err := th.ErrIfNoHelm(); err != nil { + t.Skip("skipping: " + err.Error()) + } + + rm := th.LoadAndRunGenerator(` +apiVersion: builtin +kind: HelmChartInflationGenerator +metadata: + name: terraform +name: terraform +version: 1.0.0 +repo: https://helm.releases.hashicorp.com +releaseName: terraforming-mars +valuesInline: + global: + enabled: false + tests: + enabled: false +`) + th.AssertActualEqualsExpected(rm, "") +} diff --git a/plugin/builtin/helmchartinflationgenerator/include_crds_testdata.txt b/plugin/builtin/helmchartinflationgenerator/include_crds_testdata.txt new file mode 100644 index 000000000..9754fb802 --- /dev/null +++ b/plugin/builtin/helmchartinflationgenerator/include_crds_testdata.txt @@ -0,0 +1,262 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.3.0 + creationTimestamp: null + name: workspaces.app.terraform.io +spec: + group: app.terraform.io + names: + kind: Workspace + listKind: WorkspaceList + plural: workspaces + singular: workspace + scope: Namespaced + subresources: + status: {} + validation: + openAPIV3Schema: + description: Workspace is the Schema for the workspaces API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: WorkspaceSpec defines the desired state of Workspace + properties: + agentPoolID: + description: Specifies the agent pool ID we wish to use. + type: string + module: + description: Module source and version to use + nullable: true + properties: + source: + description: Any remote module source (version control, registry) + type: string + version: + description: Module version for registry modules + type: string + required: + - source + type: object + organization: + description: Terraform Cloud organization + type: string + outputs: + description: Outputs denote outputs wanted + items: + description: OutputSpec specifies which values need to be output + properties: + key: + description: Output name + type: string + moduleOutputName: + description: Attribute name in module + type: string + type: object + type: array + secretsMountPath: + description: File path within operator pod to load workspace secrets + type: string + sshKeyID: + description: SSH Key ID. This key must already exist in the TF Cloud + organization. This can either be the user assigned name of the SSH + Key, or the system assigned ID. + type: string + terraformVersion: + description: Terraform version used for this workspace. The default + is `latest`. + type: string + variables: + description: Variables as inputs to module + items: + description: Variable denotes an input to the module + properties: + environmentVariable: + description: EnvironmentVariable denotes if this variable should + be created as environment variable + type: boolean + hcl: + description: String input should be an HCL-formatted variable + type: boolean + key: + description: Variable name + type: string + sensitive: + description: Variable is a secret and should be retrieved from + file + type: boolean + value: + description: Variable value + type: string + valueFrom: + description: Source for the variable's value. Cannot be used if + value is not empty. + properties: + configMapKeyRef: + description: Selects a key of a ConfigMap. + properties: + key: + description: The key to select. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the ConfigMap or its key + must be defined + type: boolean + required: + - key + type: object + fieldRef: + description: 'Selects a field of the pod: supports metadata.name, + metadata.namespace, metadata.labels, metadata.annotations, + spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, + status.podIPs.' + properties: + apiVersion: + description: Version of the schema the FieldPath is written + in terms of, defaults to "v1". + type: string + fieldPath: + description: Path of the field to select in the specified + API version. + type: string + required: + - fieldPath + type: object + resourceFieldRef: + description: 'Selects a resource of the container: only resources + limits and requests (limits.cpu, limits.memory, limits.ephemeral-storage, + requests.cpu, requests.memory and requests.ephemeral-storage) + are currently supported.' + properties: + containerName: + description: 'Container name: required for volumes, optional + for env vars' + type: string + divisor: + anyOf: + - type: integer + - type: string + description: Specifies the output format of the exposed + resources, defaults to "1" + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + resource: + description: 'Required: resource to select' + type: string + required: + - resource + type: object + secretKeyRef: + description: Selects a key of a secret in the pod's namespace + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the Secret or its key must + be defined + type: boolean + required: + - key + type: object + type: object + required: + - environmentVariable + - key + - sensitive + type: object + type: array + vcs: + description: Details of the VCS repository we want to connect to the + workspace + nullable: true + properties: + branch: + description: The repository branch to use + type: string + ingress_submodules: + description: Whether submodules should be fetched when cloning the + VCS repository (Defaults to false) + type: boolean + repo_identifier: + description: A reference to your VCS repository in the format org/repo + type: string + token_id: + description: Token ID of the VCS Connection (OAuth Connection Token) + to use https://www.terraform.io/docs/cloud/vcs + type: string + required: + - repo_identifier + - token_id + type: object + required: + - organization + - secretsMountPath + type: object + status: + description: WorkspaceStatus defines the observed state of Workspace + properties: + configVersionID: + description: Configuration Version ID + type: string + outputs: + description: Outputs from state file + items: + description: OutputStatus outputs the values of Terraform output + properties: + key: + description: Attribute name in module + type: string + value: + description: Value + type: string + type: object + type: array + runID: + description: Run ID + type: string + runStatus: + description: Run Status gets the run status + type: string + workspaceID: + description: Workspace ID + type: string + required: + - configVersionID + - runID + - runStatus + - workspaceID + type: object + type: object + version: v1alpha1 + versions: + - name: v1alpha1 + served: true + storage: true +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] From 73da51d0ac4a9257d57d09d2a15a4811a570b210 Mon Sep 17 00:00:00 2001 From: Devon Mizelle Date: Mon, 17 May 2021 12:18:33 -0400 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Steven E. Harris --- api/types/helmchartargs.go | 2 +- examples/chart.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/types/helmchartargs.go b/api/types/helmchartargs.go index 5f4fc622d..c0fff2457 100644 --- a/api/types/helmchartargs.go +++ b/api/types/helmchartargs.go @@ -69,7 +69,7 @@ type HelmChart struct { // Defaults to 'override'. ValuesMerge string `json:"valuesMerge,omitempty" yaml:"valuesMerge,omitempty"` - // InstallCRDs specifies if Helm should also generate CustomResourceDefinitions. + // IncludeCRDs specifies if Helm should also generate CustomResourceDefinitions. // Defaults to 'false'. IncludeCRDs bool `json:"includeCRDs,omitempty" yaml:"includeCRDs,omitempty"` } diff --git a/examples/chart.md b/examples/chart.md index fcaa94021..0e84074b7 100644 --- a/examples/chart.md +++ b/examples/chart.md @@ -108,7 +108,7 @@ The `valuesInline` field overrides some native chart values. The `includeCRDs` field instructs Helm to generate `CustomResourceDefinitions`. -See the [Helm docs for further info](https://helm.sh/docs/chart_best_practices/custom_resource_definitions/) +See [the Helm documentation](https://helm.sh/docs/chart_best_practices/custom_resource_definitions/) for details. Check the directory layout: