From 908666062e178a2ec4ea0e5176a471c657c2ac21 Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Fri, 6 Jan 2023 17:36:44 -0500 Subject: [PATCH 01/15] feat: Added ability to load terraform registry url and credentials form a kubernetes secret. Signed-off-by: raradhakrishnan --- api/types/state.go | 1 + api/v1beta1/configuration_types.go | 3 + api/v1beta2/configuration_types.go | 3 + ...terraform.core.oam.dev_configurations.yaml | 28 ++++++ controllers/configuration_controller.go | 92 +++++++++++++++++++ controllers/configuration_controller_test.go | 41 ++++++++- 6 files changed, 167 insertions(+), 1 deletion(-) diff --git a/api/types/state.go b/api/types/state.go index 5a1072f5..151e8d9a 100644 --- a/api/types/state.go +++ b/api/types/state.go @@ -35,6 +35,7 @@ const ( InvalidRegion ConfigurationState = "InvalidRegion" TerraformInitError ConfigurationState = "TerraformInitError" InvalidGitCredentialsSecretReference ConfigurationState = "InvalidGitCredentialsSecretReference" + InvalidTerraformCredentialsSecretReference ConfigurationState = "InvalidTerraformCredentialsSecretReference" ) // Stage is the Terraform stage diff --git a/api/v1beta1/configuration_types.go b/api/v1beta1/configuration_types.go index 060cb2a5..c23b4f66 100644 --- a/api/v1beta1/configuration_types.go +++ b/api/v1beta1/configuration_types.go @@ -51,6 +51,9 @@ type ConfigurationSpec struct { // GitCredentialsSecretReference specifies the reference to the secret containing the git credentials GitCredentialsSecretReference *v1.SecretReference `json:"gitCredentialsSecretReference,omitempty"` + + // TerraformCredentialsSecretReference specifies the reference to the secret containing the terraform credentials and terraform registry details + TerraformCredentialsSecretReference *v1.SecretReference `json:"terraformCredentialsSecretReference,omitempty"` } // BaseConfigurationSpec defines the common fields of a ConfigurationSpec diff --git a/api/v1beta2/configuration_types.go b/api/v1beta2/configuration_types.go index 146e8ddf..e234d587 100644 --- a/api/v1beta2/configuration_types.go +++ b/api/v1beta2/configuration_types.go @@ -79,6 +79,9 @@ type ConfigurationSpec struct { // GitCredentialsSecretReference specifies the reference to the secret containing the git credentials GitCredentialsSecretReference *v1.SecretReference `json:"gitCredentialsSecretReference,omitempty"` + + // TerraformCredentialsSecretReference specifies the reference to the secret containing the terraform credentials and terraform registry details + TerraformCredentialsSecretReference *v1.SecretReference `json:"terraformCredentialsSecretReference,omitempty"` } // ConfigurationStatus defines the observed state of Configuration diff --git a/chart/crds/terraform.core.oam.dev_configurations.yaml b/chart/crds/terraform.core.oam.dev_configurations.yaml index 8b964b24..fc8d1239 100644 --- a/chart/crds/terraform.core.oam.dev_configurations.yaml +++ b/chart/crds/terraform.core.oam.dev_configurations.yaml @@ -111,6 +111,20 @@ spec: description: Remote is a git repo which contains hcl files. Currently, only public git repos are supported. type: string + terraformCredentialsSecretReference: + description: TerraformCredentialsSecretReference specifies the reference + to the secret containing the terraform credentials and terraform + registry details + properties: + name: + description: Name is unique within a namespace to reference a + secret resource. + type: string + namespace: + description: Namespace defines the space within which the secret + name must be unique. + type: string + type: object variable: type: object x-kubernetes-preserve-unknown-fields: true @@ -318,6 +332,20 @@ spec: description: Remote is a git repo which contains hcl files. Currently, only public git repos are supported. type: string + terraformCredentialsSecretReference: + description: TerraformCredentialsSecretReference specifies the reference + to the secret containing the terraform credentials and terraform + registry details + properties: + name: + description: Name is unique within a namespace to reference a + secret resource. + type: string + namespace: + description: Namespace defines the space within which the secret + name must be unique. + type: string + type: object variable: type: object x-kubernetes-preserve-unknown-fields: true diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 83a2695a..d4fd2db7 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -69,6 +69,11 @@ const ( GitAuthConfigVolumeName = "git-auth-configuration" // GitAuthConfigVolumeMountPath is the volume mount path for git auth configurtaion GitAuthConfigVolumeMountPath = "/root/.ssh" + // TerraformCredentialsConfigVolumeName is the volume name for terraform auth configurtaion + TerraformCredentialsConfigVolumeName = "terraform-credentials-configuration" + // TerraformCredentialsConfigVolumeMountPath is the volume mount path for terraform auth configurtaion + TerraformCredentialsConfigVolumeMountPath = "/root/.terraform.d" + ) const ( @@ -98,6 +103,10 @@ const ( const ( GitCredsKnownHosts = "known_hosts" + // Terraform credentials + TerraformCredentials = "credentials.tfrc.json" + // Terraform Registry + TerraformRegistry = "terraformrc" ) // ConfigurationReconciler reconciles a Configuration object. @@ -257,6 +266,7 @@ type TFConfigurationMeta struct { Credentials map[string]string JobEnv map[string]interface{} GitCredentialsSecretReference *v1.SecretReference + TerraformCredentialsSecretReference *v1.SecretReference Backend backend.Backend // JobNodeSelector Expose the node selector of job to the controller level @@ -322,6 +332,10 @@ func initTFConfigurationMeta(req ctrl.Request, configuration v1beta2.Configurati meta.GitCredentialsSecretReference = configuration.Spec.GitCredentialsSecretReference } + if configuration.Spec.TerraformCredentialsSecretReference != nil { + meta.TerraformCredentialsSecretReference = configuration.Spec.TerraformCredentialsSecretReference + } + return meta } @@ -571,6 +585,20 @@ func (r *ConfigurationReconciler) preCheck(ctx context.Context, configuration *v } } + if meta.TerraformCredentialsSecretReference != nil { + terraformCreds, err := GetTerraformCredentialsSecret(ctx, k8sClient, meta.TerraformCredentialsSecretReference) + if terraformCreds == nil { + msg := string(types.InvalidTerraformCredentialsSecretReference) + if err != nil { + msg = err.Error() + } + if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.InvalidTerraformCredentialsSecretReference, msg); updateStatusErr != nil { + return errors.Wrap(updateStatusErr, msg) + } + return errors.New(msg) + } + } + // Render configuration with backend completeConfiguration, backendConf, err := meta.RenderConfiguration(configuration, configurationType) if err != nil { @@ -786,6 +814,32 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe }) } + if meta.TerraformCredentialsSecretReference != nil { + initContainerVolumeMounts = append(initContainerVolumeMounts, + v1.VolumeMount{ + Name: TerraformCredentialsConfigVolumeName, + MountPath: TerraformCredentialsConfigVolumeMountPath, + }) + + // copy the terraformrc file from /root/terraform.d/ to /root + copyCommand = "cp /root/.terraform.d/terraformrc /root/.terraformrc" + + command := []string{ + "sh", + "-c", + copyCommand, + } + + initContainers = append(initContainers, + v1.Container{ + Name: "terraform-credentials-configuration", + Image: meta.GitImage, + ImagePullPolicy: v1.PullIfNotPresent, + Command: command, + VolumeMounts: initContainerVolumeMounts, + }) + } + // run `terraform init` tfPreApplyInitContainer = v1.Container{ Name: terraformInitContainerName, @@ -901,6 +955,10 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume { gitAuthConfigVolume := meta.createGitAuthConfigVolume() executorVolumes = append(executorVolumes, gitAuthConfigVolume) } + if meta.terraformCredentialsSecretReference != nil { + terraformCredentialsConfigVolume := meta.createTerraformCredentialsConfigVolume() + executorVolumes = append(executorVolumes, terraformCredentialsConfigVolume) + } return executorVolumes } @@ -929,6 +987,16 @@ func (meta *TFConfigurationMeta) createGitAuthConfigVolume() v1.Volume { return gitAuthSecretVolume } +func (meta *TFConfigurationMeta) createTerraformCredentialsConfigVolume() v1.Volume { + var terraformSecretDefaultMode int32 = 0400 + terraformCredentialsSecretVolumeSource := v1.SecretVolumeSource{} + terraformCredentialsSecretVolumeSource.SecretName = meta.TerraformCredentialsSecretReference.Name + terraformCredentialsSecretVolumeSource.DefaultMode = &terraformSecretDefaultMode + terraformCredentialsSecretVolume := v1.Volume{Name: TerraformCredentialsConfigVolumeName} + terraformCredentialsSecretVolume.Secret = &terraformCredentialsSecretVolumeSource + return terraformCredentialsSecretVolume +} + // TfStateProperty is the tf state property for an output type TfStateProperty struct { Value interface{} `json:"value,omitempty"` @@ -1358,3 +1426,27 @@ func GetGitCredentialsSecret(ctx context.Context, k8sClient client.Client, secre return secret, nil } + + +// GetTerraformCredentialsSecret will get the secret containing the terraform credentials and terrform registry details +func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, secretRef *v1.SecretReference) (*v1.Secret, error){ + secret := &v1.Secret{} + namespacedName := client.ObjectKey{Name: secretRef.Name, Namespace: secretRef.Namespace} + err := k8sClient.Get(ctx, namespacedName, secret) + if err != nil { + errMsg := "Failed to get terraform credentials secret" + klog.ErrorS(err, errMsg, "Name", secretRef.Name, "Namespace", secretRef.Namespace) + return nil, errors.Wrap(err, errMsg) + } + + needSecretKeys := []string{TerraformRegistry, TerraformCredentials} + for _, key := range needSecretKeys { + if _, ok := secret.Data[key]; !ok { + err := errors.Errorf("'%s' not in git credentials secret", key) + return nil, err + } + } + + return secret, nil + +} diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 68ff702e..65a6612c 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1618,7 +1618,7 @@ func TestAssembleTerraformJobWithGitCredentialsSecretRef(t *testing.T) { GitCredentialsSecretReference: &corev1.SecretReference{ Namespace: "default", Name: "git-ssh", - }, + } } job := meta.assembleTerraformJob(TerraformApply) @@ -1639,6 +1639,45 @@ func TestAssembleTerraformJobWithGitCredentialsSecretRef(t *testing.T) { assert.Contains(t, spec.Volumes, gitAuthSecretVolume) } +func TestAssembleTerraformJobWithTerraformCredentialsSecretRef(t *testing.T) { + meta := &TFConfigurationMeta{ + Name: "a", + ConfigurationCMName: "b", + BusyboxImage: "c", + GitImage: "d", + Namespace: "e", + TerraformImage: "f", + RemoteGit: "g", + GitCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "git-ssh", + }, + terraformCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-credentials", + }, + } + + job := meta.assembleTerraformJob(TerraformApply) + spec := job.Spec.Template.Spec + + var terraformSecretDefaultMode int32 = 0400 + terraformCredentialsSecretVolume := corev1.Volume{Name: TerraformCredentialsConfigVolumeName} + terraformCredentialsSecretVolume.Secret = &corev1.SecretVolumeSource{ + SecretName: "terraform-credentials", + DefaultMode: &gitSecretDefaultMode, + } + + terraformCredentialsSecretVolumeMount := corev1.VolumeMount{ + Name: TerraformCredentialsConfigVolumeName, + MountPath: TerraformCredentialsConfigVolumeMountPath, + } + + assert.Contains(t, spec.InitContainers[1].VolumeMounts, terraformCredentialsSecretVolumeMount) + assert.Contains(t, spec.Volumes, terraformCredentialsSecretVolume) + +} + func TestTfStatePropertyToToProperty(t *testing.T) { testcases := []TfStateProperty{ { From 2ac8852895513439f28fdb2b521155fa02561b30 Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Mon, 9 Jan 2023 11:18:36 -0500 Subject: [PATCH 02/15] feat: fixed review comments. Signed-off-by: raradhakrishnan --- controllers/configuration_controller.go | 2 +- controllers/configuration_controller_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index d4fd2db7..e9dcb60f 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -1442,7 +1442,7 @@ func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, needSecretKeys := []string{TerraformRegistry, TerraformCredentials} for _, key := range needSecretKeys { if _, ok := secret.Data[key]; !ok { - err := errors.Errorf("'%s' not in git credentials secret", key) + err := errors.Errorf("'%s' not in terraform credentials secret", key) return nil, err } } diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 65a6612c..2ec5a54d 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1665,7 +1665,7 @@ func TestAssembleTerraformJobWithTerraformCredentialsSecretRef(t *testing.T) { terraformCredentialsSecretVolume := corev1.Volume{Name: TerraformCredentialsConfigVolumeName} terraformCredentialsSecretVolume.Secret = &corev1.SecretVolumeSource{ SecretName: "terraform-credentials", - DefaultMode: &gitSecretDefaultMode, + DefaultMode: &terraformSecretDefaultMode, } terraformCredentialsSecretVolumeMount := corev1.VolumeMount{ From c9fff278ebfdbeb88ac50ab6f7a2c522ccbd4d51 Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Mon, 9 Jan 2023 14:18:19 -0500 Subject: [PATCH 03/15] feat: added required unit tests. Signed-off-by: raradhakrishnan --- controllers/configuration_controller.go | 55 ++++---- controllers/configuration_controller_test.go | 139 ++++++++++++++++++- 2 files changed, 163 insertions(+), 31 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index e9dcb60f..3ec7c72b 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -73,7 +73,6 @@ const ( TerraformCredentialsConfigVolumeName = "terraform-credentials-configuration" // TerraformCredentialsConfigVolumeMountPath is the volume mount path for terraform auth configurtaion TerraformCredentialsConfigVolumeMountPath = "/root/.terraform.d" - ) const ( @@ -245,27 +244,27 @@ type ResourceQuota struct { // TFConfigurationMeta is all the metadata of a Configuration type TFConfigurationMeta struct { - Name string - Namespace string - ControllerNamespace string - ConfigurationType types.ConfigurationType - CompleteConfiguration string - RemoteGit string - RemoteGitPath string - ConfigurationChanged bool - EnvChanged bool - ConfigurationCMName string - ApplyJobName string - DestroyJobName string - Envs []v1.EnvVar - ProviderReference *crossplane.Reference - VariableSecretName string - VariableSecretData map[string][]byte - DeleteResource bool - Region string - Credentials map[string]string - JobEnv map[string]interface{} - GitCredentialsSecretReference *v1.SecretReference + Name string + Namespace string + ControllerNamespace string + ConfigurationType types.ConfigurationType + CompleteConfiguration string + RemoteGit string + RemoteGitPath string + ConfigurationChanged bool + EnvChanged bool + ConfigurationCMName string + ApplyJobName string + DestroyJobName string + Envs []v1.EnvVar + ProviderReference *crossplane.Reference + VariableSecretName string + VariableSecretData map[string][]byte + DeleteResource bool + Region string + Credentials map[string]string + JobEnv map[string]interface{} + GitCredentialsSecretReference *v1.SecretReference TerraformCredentialsSecretReference *v1.SecretReference Backend backend.Backend @@ -822,7 +821,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe }) // copy the terraformrc file from /root/terraform.d/ to /root - copyCommand = "cp /root/.terraform.d/terraformrc /root/.terraformrc" + copyCommand := "cp /root/.terraform.d/terraformrc /root/.terraformrc" command := []string{ "sh", @@ -832,7 +831,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe initContainers = append(initContainers, v1.Container{ - Name: "terraform-credentials-configuration", + Name: "terraform-creds-configuration", Image: meta.GitImage, ImagePullPolicy: v1.PullIfNotPresent, Command: command, @@ -955,10 +954,11 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume { gitAuthConfigVolume := meta.createGitAuthConfigVolume() executorVolumes = append(executorVolumes, gitAuthConfigVolume) } - if meta.terraformCredentialsSecretReference != nil { + if meta.TerraformCredentialsSecretReference != nil { terraformCredentialsConfigVolume := meta.createTerraformCredentialsConfigVolume() executorVolumes = append(executorVolumes, terraformCredentialsConfigVolume) } + return executorVolumes } @@ -1427,11 +1427,10 @@ func GetGitCredentialsSecret(ctx context.Context, k8sClient client.Client, secre return secret, nil } - // GetTerraformCredentialsSecret will get the secret containing the terraform credentials and terrform registry details -func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, secretRef *v1.SecretReference) (*v1.Secret, error){ +func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, secretRef *v1.SecretReference) (*v1.Secret, error) { secret := &v1.Secret{} - namespacedName := client.ObjectKey{Name: secretRef.Name, Namespace: secretRef.Namespace} + namespacedName := client.ObjectKey{Name: secretRef.Name, Namespace: secretRef.Namespace} err := k8sClient.Get(ctx, namespacedName, secret) if err != nil { errMsg := "Failed to get terraform credentials secret" diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 2ec5a54d..9aa5c221 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1618,7 +1618,7 @@ func TestAssembleTerraformJobWithGitCredentialsSecretRef(t *testing.T) { GitCredentialsSecretReference: &corev1.SecretReference{ Namespace: "default", Name: "git-ssh", - } + }, } job := meta.assembleTerraformJob(TerraformApply) @@ -1652,7 +1652,7 @@ func TestAssembleTerraformJobWithTerraformCredentialsSecretRef(t *testing.T) { Namespace: "default", Name: "git-ssh", }, - terraformCredentialsSecretReference: &corev1.SecretReference{ + TerraformCredentialsSecretReference: &corev1.SecretReference{ Namespace: "default", Name: "terraform-credentials", }, @@ -1673,7 +1673,7 @@ func TestAssembleTerraformJobWithTerraformCredentialsSecretRef(t *testing.T) { MountPath: TerraformCredentialsConfigVolumeMountPath, } - assert.Contains(t, spec.InitContainers[1].VolumeMounts, terraformCredentialsSecretVolumeMount) + assert.Contains(t, spec.InitContainers[2].VolumeMounts, terraformCredentialsSecretVolumeMount) assert.Contains(t, spec.Volumes, terraformCredentialsSecretVolume) } @@ -2697,3 +2697,136 @@ func TestCheckGitCredentialsSecretReference(t *testing.T) { }) } } + +func TestCheckTerraformCredentialsSecretReference(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + corev1.AddToScheme(scheme) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + credentialstfrcjson := []byte("tfcreds") + terraformrc := []byte("tfrc") + + secret := &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Name: "terraform-creds", + }, + Data: map[string][]byte{ + "credentials.tfrc.json": credentialstfrcjson, + "terraformrc": terraformrc, + }, + Type: corev1.SecretTypeSSHAuth, + } + assert.Nil(t, k8sClient.Create(ctx, secret)) + assert.Nil(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(secret), secret)) + + secretNotTerraformCreds := &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Name: "terraform-creds-no-creds", + }, + Data: map[string][]byte{ + "terraformrc": terraformrc, + }, + Type: corev1.SecretTypeSSHAuth, + } + + assert.Nil(t, k8sClient.Create(ctx, secretNotTerraformCreds)) + + secretNotTerraformRc := &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Name: "terraform-creds-no-terraformrc", + }, + Data: map[string][]byte{ + "credentials.tfrc.json": credentialstfrcjson, + }, + Type: corev1.SecretTypeSSHAuth, + } + + assert.Nil(t, k8sClient.Create(ctx, secretNotTerraformRc)) + + type args struct { + k8sClient client.Client + TerraformCredentialsSecretReference *corev1.SecretReference + } + + type want struct { + secret *corev1.Secret + errMsg string + } + + testcases := []struct { + name string + args args + want want + }{ + { + name: "secret not found", + args: args{ + k8sClient: k8sClient, + TerraformCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-credentials", + }, + }, + want: want{ + errMsg: "Failed to get terraform credentials secret: secrets \"terraform-credentials\" not found", + }, + }, + { + name: "key 'terraformrc' not in terraform credentials secret", + args: args{ + k8sClient: k8sClient, + TerraformCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-creds-no-terraformrc", + }, + }, + want: want{ + errMsg: fmt.Sprintf("'%s' not in terraform credentials secret", TerraformRegistry), + }, + }, + { + name: "key 'credentials.tfrc.json' not in terraform credentials secret", + args: args{ + k8sClient: k8sClient, + TerraformCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-creds-no-creds", + }, + }, + want: want{ + errMsg: fmt.Sprintf("'%s' not in terraform credentials secret", TerraformCredentials), + }, + }, + { + name: "secret exists", + args: args{ + k8sClient: k8sClient, + TerraformCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-creds", + }, + }, + want: want{ + secret: secret, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + sec, err := GetTerraformCredentialsSecret(ctx, tc.args.k8sClient, tc.args.TerraformCredentialsSecretReference) + + if err != nil { + assert.EqualError(t, err, tc.want.errMsg) + } + if tc.want.secret != nil { + assert.EqualValues(t, sec, tc.want.secret) + } + }) + } + +} From 57bb6cda32fc0632f6e87d78ffac6d5b365cc992 Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Mon, 9 Jan 2023 14:19:43 -0500 Subject: [PATCH 04/15] feat: autogenerated files during test. Signed-off-by: raradhakrishnan --- api/v1beta1/zz_generated.deepcopy.go | 5 +++++ api/v1beta2/zz_generated.deepcopy.go | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 21d9e64b..f86e4967 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -182,6 +182,11 @@ func (in *ConfigurationSpec) DeepCopyInto(out *ConfigurationSpec) { *out = new(v1.SecretReference) **out = **in } + if in.TerraformCredentialsSecretReference != nil { + in, out := &in.TerraformCredentialsSecretReference, &out.TerraformCredentialsSecretReference + *out = new(v1.SecretReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigurationSpec. diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 64795fa0..5b72f0db 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -191,6 +191,11 @@ func (in *ConfigurationSpec) DeepCopyInto(out *ConfigurationSpec) { *out = new(v1.SecretReference) **out = **in } + if in.TerraformCredentialsSecretReference != nil { + in, out := &in.TerraformCredentialsSecretReference, &out.TerraformCredentialsSecretReference + *out = new(v1.SecretReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigurationSpec. From 52b365108c49a157417bac6b05ec943e2f00452a Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Mon, 9 Jan 2023 17:06:17 -0500 Subject: [PATCH 05/15] feat: fixed intendation. Signed-off-by: raradhakrishnan --- controllers/configuration_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 3ec7c72b..677d6617 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -958,7 +958,6 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume { terraformCredentialsConfigVolume := meta.createTerraformCredentialsConfigVolume() executorVolumes = append(executorVolumes, terraformCredentialsConfigVolume) } - return executorVolumes } @@ -1427,6 +1426,7 @@ func GetGitCredentialsSecret(ctx context.Context, k8sClient client.Client, secre return secret, nil } + // GetTerraformCredentialsSecret will get the secret containing the terraform credentials and terrform registry details func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, secretRef *v1.SecretReference) (*v1.Secret, error) { secret := &v1.Secret{} From ee99851f0dc195d85b9ff2f30f7cfffab8374c3e Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Thu, 12 Jan 2023 11:04:23 -0500 Subject: [PATCH 06/15] feat: added ability to volume mount .terraformrc and terraform-credentials-helper form configmaps. Signed-off-by: raradhakrishnan --- api/types/state.go | 32 +- api/v1beta1/configuration_types.go | 8 +- api/v1beta1/zz_generated.deepcopy.go | 10 + api/v1beta2/configuration_types.go | 6 + api/v1beta2/zz_generated.deepcopy.go | 10 + ...terraform.core.oam.dev_configurations.yaml | 57 +++- controllers/configuration_controller.go | 212 +++++++++---- controllers/configuration_controller_test.go | 285 ++++++++++++++++-- 8 files changed, 519 insertions(+), 101 deletions(-) diff --git a/api/types/state.go b/api/types/state.go index 151e8d9a..4adea760 100644 --- a/api/types/state.go +++ b/api/types/state.go @@ -21,21 +21,23 @@ type ConfigurationState string // Reasons a resource is or is not ready. const ( - Authorizing ConfigurationState = "Authorizing" - ProviderNotFound ConfigurationState = "ProviderNotFound" - ProviderNotReady ConfigurationState = "ProviderNotReady" - ConfigurationStaticCheckFailed ConfigurationState = "ConfigurationSpecNotValid" - Available ConfigurationState = "Available" - ConfigurationProvisioningAndChecking ConfigurationState = "ProvisioningAndChecking" - ConfigurationDestroying ConfigurationState = "Destroying" - ConfigurationApplyFailed ConfigurationState = "ApplyFailed" - ConfigurationDestroyFailed ConfigurationState = "DestroyFailed" - ConfigurationReloading ConfigurationState = "ConfigurationReloading" - GeneratingOutputs ConfigurationState = "GeneratingTerraformOutputs" - InvalidRegion ConfigurationState = "InvalidRegion" - TerraformInitError ConfigurationState = "TerraformInitError" - InvalidGitCredentialsSecretReference ConfigurationState = "InvalidGitCredentialsSecretReference" - InvalidTerraformCredentialsSecretReference ConfigurationState = "InvalidTerraformCredentialsSecretReference" + Authorizing ConfigurationState = "Authorizing" + ProviderNotFound ConfigurationState = "ProviderNotFound" + ProviderNotReady ConfigurationState = "ProviderNotReady" + ConfigurationStaticCheckFailed ConfigurationState = "ConfigurationSpecNotValid" + Available ConfigurationState = "Available" + ConfigurationProvisioningAndChecking ConfigurationState = "ProvisioningAndChecking" + ConfigurationDestroying ConfigurationState = "Destroying" + ConfigurationApplyFailed ConfigurationState = "ApplyFailed" + ConfigurationDestroyFailed ConfigurationState = "DestroyFailed" + ConfigurationReloading ConfigurationState = "ConfigurationReloading" + GeneratingOutputs ConfigurationState = "GeneratingTerraformOutputs" + InvalidRegion ConfigurationState = "InvalidRegion" + TerraformInitError ConfigurationState = "TerraformInitError" + InvalidGitCredentialsSecretReference ConfigurationState = "InvalidGitCredentialsSecretReference" + InvalidTerraformCredentialsSecretReference ConfigurationState = "InvalidTerraformCredentialsSecretReference" + InvalidTerraformRegistryConfigMapReference ConfigurationState = "InvalidTerraformRegistryConfigMapReference" + InvalidTerraformCredentialsHelperConfigMapReference ConfigurationState = "InvalidTerraformCredentialsHelperConfigMapReference" ) // Stage is the Terraform stage diff --git a/api/v1beta1/configuration_types.go b/api/v1beta1/configuration_types.go index c23b4f66..6e2cd5ce 100644 --- a/api/v1beta1/configuration_types.go +++ b/api/v1beta1/configuration_types.go @@ -52,8 +52,14 @@ type ConfigurationSpec struct { // GitCredentialsSecretReference specifies the reference to the secret containing the git credentials GitCredentialsSecretReference *v1.SecretReference `json:"gitCredentialsSecretReference,omitempty"` - // TerraformCredentialsSecretReference specifies the reference to the secret containing the terraform credentials and terraform registry details + // TerraformCredentialsSecretReference specifies the reference to the secret containing the terraform credentials TerraformCredentialsSecretReference *v1.SecretReference `json:"terraformCredentialsSecretReference,omitempty"` + + // TerraformRegistryConfigMapReference specifies the reference to a config map containing the terraform registry configuration + TerraformRegistryConfigMapReference *v1.SecretReference `json:"terraformRegistryConfigMapReference,omitempty"` + + // TerraformCredentialsHelperConfigMapReference specifies the reference to a configmap containing the terraform registry credentials helper + TerraformCredentialsHelperConfigMapReference *v1.SecretReference `json:"terraformCredentialsHelperConfigMapReference,omitempty"` } // BaseConfigurationSpec defines the common fields of a ConfigurationSpec diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index f86e4967..3ae47a08 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -187,6 +187,16 @@ func (in *ConfigurationSpec) DeepCopyInto(out *ConfigurationSpec) { *out = new(v1.SecretReference) **out = **in } + if in.TerraformRegistryConfigMapReference != nil { + in, out := &in.TerraformRegistryConfigMapReference, &out.TerraformRegistryConfigMapReference + *out = new(v1.SecretReference) + **out = **in + } + if in.TerraformCredentialsHelperConfigMapReference != nil { + in, out := &in.TerraformCredentialsHelperConfigMapReference, &out.TerraformCredentialsHelperConfigMapReference + *out = new(v1.SecretReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigurationSpec. diff --git a/api/v1beta2/configuration_types.go b/api/v1beta2/configuration_types.go index e234d587..d7ff2763 100644 --- a/api/v1beta2/configuration_types.go +++ b/api/v1beta2/configuration_types.go @@ -82,6 +82,12 @@ type ConfigurationSpec struct { // TerraformCredentialsSecretReference specifies the reference to the secret containing the terraform credentials and terraform registry details TerraformCredentialsSecretReference *v1.SecretReference `json:"terraformCredentialsSecretReference,omitempty"` + + // TerraformRegistryConfigMapReference specifies the reference to a config map containing the terraform registry configuration + TerraformRegistryConfigMapReference *v1.SecretReference `json:"terraformRegistryConfigMapReference,omitempty"` + + // TerraformCredentialsHelperConfigMapReference specifies the reference to a configmap containing the terraform registry credentials helper + TerraformCredentialsHelperConfigMapReference *v1.SecretReference `json:"terraformCredentialsHelperConfigMapReference,omitempty"` } // ConfigurationStatus defines the observed state of Configuration diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 5b72f0db..0904cb8f 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -196,6 +196,16 @@ func (in *ConfigurationSpec) DeepCopyInto(out *ConfigurationSpec) { *out = new(v1.SecretReference) **out = **in } + if in.TerraformRegistryConfigMapReference != nil { + in, out := &in.TerraformRegistryConfigMapReference, &out.TerraformRegistryConfigMapReference + *out = new(v1.SecretReference) + **out = **in + } + if in.TerraformCredentialsHelperConfigMapReference != nil { + in, out := &in.TerraformCredentialsHelperConfigMapReference, &out.TerraformCredentialsHelperConfigMapReference + *out = new(v1.SecretReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigurationSpec. diff --git a/chart/crds/terraform.core.oam.dev_configurations.yaml b/chart/crds/terraform.core.oam.dev_configurations.yaml index fc8d1239..c5cacec3 100644 --- a/chart/crds/terraform.core.oam.dev_configurations.yaml +++ b/chart/crds/terraform.core.oam.dev_configurations.yaml @@ -111,10 +111,36 @@ spec: description: Remote is a git repo which contains hcl files. Currently, only public git repos are supported. type: string + terraformCredentialsHelperConfigMapReference: + description: TerraformCredentialsHelperConfigMapReference specifies + the reference to a configmap containing the terraform registry credentials + helper + properties: + name: + description: Name is unique within a namespace to reference a + secret resource. + type: string + namespace: + description: Namespace defines the space within which the secret + name must be unique. + type: string + type: object terraformCredentialsSecretReference: description: TerraformCredentialsSecretReference specifies the reference - to the secret containing the terraform credentials and terraform - registry details + to the secret containing the terraform credentials + properties: + name: + description: Name is unique within a namespace to reference a + secret resource. + type: string + namespace: + description: Namespace defines the space within which the secret + name must be unique. + type: string + type: object + terraformRegistryConfigMapReference: + description: TerraformRegistryConfigMapReference specifies the reference + to a config map containing the terraform registry configuration properties: name: description: Name is unique within a namespace to reference a @@ -332,6 +358,20 @@ spec: description: Remote is a git repo which contains hcl files. Currently, only public git repos are supported. type: string + terraformCredentialsHelperConfigMapReference: + description: TerraformCredentialsHelperConfigMapReference specifies + the reference to a configmap containing the terraform registry credentials + helper + properties: + name: + description: Name is unique within a namespace to reference a + secret resource. + type: string + namespace: + description: Namespace defines the space within which the secret + name must be unique. + type: string + type: object terraformCredentialsSecretReference: description: TerraformCredentialsSecretReference specifies the reference to the secret containing the terraform credentials and terraform @@ -346,6 +386,19 @@ spec: name must be unique. type: string type: object + terraformRegistryConfigMapReference: + description: TerraformRegistryConfigMapReference specifies the reference + to a config map containing the terraform registry configuration + properties: + name: + description: Name is unique within a namespace to reference a + secret resource. + type: string + namespace: + description: Namespace defines the space within which the secret + name must be unique. + type: string + type: object variable: type: object x-kubernetes-preserve-unknown-fields: true diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 677d6617..b58f2df9 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -73,6 +73,14 @@ const ( TerraformCredentialsConfigVolumeName = "terraform-credentials-configuration" // TerraformCredentialsConfigVolumeMountPath is the volume mount path for terraform auth configurtaion TerraformCredentialsConfigVolumeMountPath = "/root/.terraform.d" + // TerraformRegistryConfigVolumeName is the volume name of the terraform registry configuration + TerraformRegistryConfigVolumeName = "terraform-registry-configuration" + // TerraformRegistryConfigVolumeMountPath is the volume mount path for registry configuration + TerraformRegistryConfigVolumeMountPath = "/root" + // TerraformCredentialsHelperConfigVolumeName is the volume name for terraform auth configurtaion + TerraformCredentialsHelperConfigVolumeName = "terraform-credentials-helper-configuration" + // TerraformCredentialsHelperConfigVolumeMountPath is the volume mount path for terraform auth configurtaion + TerraformCredentialsHelperConfigVolumeMountPath = "/root/.terraform.d/plugins" ) const ( @@ -104,8 +112,8 @@ const ( GitCredsKnownHosts = "known_hosts" // Terraform credentials TerraformCredentials = "credentials.tfrc.json" - // Terraform Registry - TerraformRegistry = "terraformrc" + // Terraform Registry Configuration + TerraformRegistryConfig = ".terraformrc" ) // ConfigurationReconciler reconciles a Configuration object. @@ -244,28 +252,30 @@ type ResourceQuota struct { // TFConfigurationMeta is all the metadata of a Configuration type TFConfigurationMeta struct { - Name string - Namespace string - ControllerNamespace string - ConfigurationType types.ConfigurationType - CompleteConfiguration string - RemoteGit string - RemoteGitPath string - ConfigurationChanged bool - EnvChanged bool - ConfigurationCMName string - ApplyJobName string - DestroyJobName string - Envs []v1.EnvVar - ProviderReference *crossplane.Reference - VariableSecretName string - VariableSecretData map[string][]byte - DeleteResource bool - Region string - Credentials map[string]string - JobEnv map[string]interface{} - GitCredentialsSecretReference *v1.SecretReference - TerraformCredentialsSecretReference *v1.SecretReference + Name string + Namespace string + ControllerNamespace string + ConfigurationType types.ConfigurationType + CompleteConfiguration string + RemoteGit string + RemoteGitPath string + ConfigurationChanged bool + EnvChanged bool + ConfigurationCMName string + ApplyJobName string + DestroyJobName string + Envs []v1.EnvVar + ProviderReference *crossplane.Reference + VariableSecretName string + VariableSecretData map[string][]byte + DeleteResource bool + Region string + Credentials map[string]string + JobEnv map[string]interface{} + GitCredentialsSecretReference *v1.SecretReference + TerraformCredentialsSecretReference *v1.SecretReference + TerraformRegistryConfigMapReference *v1.SecretReference + TerraformCredentialsHelperConfigMapReference *v1.SecretReference Backend backend.Backend // JobNodeSelector Expose the node selector of job to the controller level @@ -335,6 +345,14 @@ func initTFConfigurationMeta(req ctrl.Request, configuration v1beta2.Configurati meta.TerraformCredentialsSecretReference = configuration.Spec.TerraformCredentialsSecretReference } + if configuration.Spec.TerraformRegistryConfigMapReference != nil { + meta.TerraformRegistryConfigMapReference = configuration.Spec.TerraformRegistryConfigMapReference + } + + if configuration.Spec.TerraformCredentialsHelperConfigMapReference != nil { + meta.TerraformCredentialsHelperConfigMapReference = configuration.Spec.TerraformCredentialsHelperConfigMapReference + } + return meta } @@ -598,6 +616,34 @@ func (r *ConfigurationReconciler) preCheck(ctx context.Context, configuration *v } } + if meta.TerraformRegistryConfigMapReference != nil { + terraformRegistryConfig, err := GetTerraformRegistryConfigMap(ctx, k8sClient, meta.TerraformRegistryConfigMapReference) + if terraformRegistryConfig == nil { + msg := string(types.InvalidTerraformRegistryConfigMapReference) + if err != nil { + msg = err.Error() + } + if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.InvalidTerraformRegistryConfigMapReference, msg); updateStatusErr != nil { + return errors.Wrap(updateStatusErr, msg) + } + return errors.New(msg) + } + } + + if meta.TerraformCredentialsHelperConfigMapReference != nil { + terraformCredentialsHelper, err := GetTerraformCredentialsHelperConfigMap(ctx, k8sClient, meta.TerraformCredentialsHelperConfigMapReference) + if terraformCredentialsHelper == nil { + msg := string(types.InvalidTerraformCredentialsHelperConfigMapReference) + if err != nil { + msg = err.Error() + } + if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.InvalidTerraformCredentialsHelperConfigMapReference, msg); updateStatusErr != nil { + return errors.Wrap(updateStatusErr, msg) + } + return errors.New(msg) + } + } + // Render configuration with backend completeConfiguration, backendConf, err := meta.RenderConfiguration(configuration, configurationType) if err != nil { @@ -765,6 +811,30 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe }, } + if meta.TerraformCredentialsSecretReference != nil { + initContainerVolumeMounts = append(initContainerVolumeMounts, + v1.VolumeMount{ + Name: TerraformCredentialsConfigVolumeName, + MountPath: TerraformCredentialsConfigVolumeMountPath, + }) + } + + if meta.TerraformRegistryConfigMapReference != nil { + initContainerVolumeMounts = append(initContainerVolumeMounts, + v1.VolumeMount{ + Name: TerraformRegistryConfigVolumeName, + MountPath: TerraformRegistryConfigVolumeMountPath, + }) + } + + if meta.TerraformCredentialsHelperConfigMapReference != nil { + initContainerVolumeMounts = append(initContainerVolumeMounts, + v1.VolumeMount{ + Name: TerraformCredentialsHelperConfigVolumeName, + MountPath: TerraformCredentialsHelperConfigVolumeMountPath, + }) + } + // prepare local Terraform .tf files initContainer = v1.Container{ Name: "prepare-input-terraform-configurations", @@ -813,32 +883,6 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe }) } - if meta.TerraformCredentialsSecretReference != nil { - initContainerVolumeMounts = append(initContainerVolumeMounts, - v1.VolumeMount{ - Name: TerraformCredentialsConfigVolumeName, - MountPath: TerraformCredentialsConfigVolumeMountPath, - }) - - // copy the terraformrc file from /root/terraform.d/ to /root - copyCommand := "cp /root/.terraform.d/terraformrc /root/.terraformrc" - - command := []string{ - "sh", - "-c", - copyCommand, - } - - initContainers = append(initContainers, - v1.Container{ - Name: "terraform-creds-configuration", - Image: meta.GitImage, - ImagePullPolicy: v1.PullIfNotPresent, - Command: command, - VolumeMounts: initContainerVolumeMounts, - }) - } - // run `terraform init` tfPreApplyInitContainer = v1.Container{ Name: terraformInitContainerName, @@ -958,6 +1002,14 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume { terraformCredentialsConfigVolume := meta.createTerraformCredentialsConfigVolume() executorVolumes = append(executorVolumes, terraformCredentialsConfigVolume) } + if meta.TerraformRegistryConfigMapReference != nil { + terraformRegistryConfigVolume := meta.createTerraformRegistryConfigVolume() + executorVolumes = append(executorVolumes, terraformRegistryConfigVolume) + } + if meta.TerraformCredentialsHelperConfigMapReference != nil { + terraformCredentialsHelperConfigVolume := meta.createTerraformCredentialsHelperConfigVolume() + executorVolumes = append(executorVolumes, terraformCredentialsHelperConfigVolume) + } return executorVolumes } @@ -996,6 +1048,26 @@ func (meta *TFConfigurationMeta) createTerraformCredentialsConfigVolume() v1.Vol return terraformCredentialsSecretVolume } +func (meta *TFConfigurationMeta) createTerraformRegistryConfigVolume() v1.Volume { + var terraformConfigMapDefaultMode int32 = 0400 + terraformRegistryConfigMapVolumeSource := v1.ConfigMapVolumeSource{} + terraformRegistryConfigMapVolumeSource.Name = meta.TerraformRegistryConfigMapReference.Name + terraformRegistryConfigMapVolumeSource.DefaultMode = &terraformConfigMapDefaultMode + terraformRegistryConfigMapVolume := v1.Volume{Name: TerraformRegistryConfigVolumeName} + terraformRegistryConfigMapVolume.ConfigMap = &terraformRegistryConfigMapVolumeSource + return terraformRegistryConfigMapVolume +} + +func (meta *TFConfigurationMeta) createTerraformCredentialsHelperConfigVolume() v1.Volume { + var terraformConfigMapDefaultMode int32 = 0400 + terraformCredentialsHelperConfigMapVolumeSource := v1.ConfigMapVolumeSource{} + terraformCredentialsHelperConfigMapVolumeSource.Name = meta.TerraformCredentialsHelperConfigMapReference.Name + terraformCredentialsHelperConfigMapVolumeSource.DefaultMode = &terraformConfigMapDefaultMode + terraformCredentialsHelperConfigMapVolume := v1.Volume{Name: TerraformCredentialsHelperConfigVolumeName} + terraformCredentialsHelperConfigMapVolume.ConfigMap = &terraformCredentialsHelperConfigMapVolumeSource + return terraformCredentialsHelperConfigMapVolume +} + // TfStateProperty is the tf state property for an output type TfStateProperty struct { Value interface{} `json:"value,omitempty"` @@ -1426,8 +1498,7 @@ func GetGitCredentialsSecret(ctx context.Context, k8sClient client.Client, secre return secret, nil } - -// GetTerraformCredentialsSecret will get the secret containing the terraform credentials and terrform registry details +// GetTerraformCredentialsSecret will get the secret containing the terraform credentials func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, secretRef *v1.SecretReference) (*v1.Secret, error) { secret := &v1.Secret{} namespacedName := client.ObjectKey{Name: secretRef.Name, Namespace: secretRef.Namespace} @@ -1438,7 +1509,7 @@ func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, return nil, errors.Wrap(err, errMsg) } - needSecretKeys := []string{TerraformRegistry, TerraformCredentials} + needSecretKeys := []string{TerraformCredentials} for _, key := range needSecretKeys { if _, ok := secret.Data[key]; !ok { err := errors.Errorf("'%s' not in terraform credentials secret", key) @@ -1449,3 +1520,38 @@ func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, return secret, nil } + +// GetTerraformRegistryConfigMap will get the config map containing the terraform registry configuration +func GetTerraformRegistryConfigMap(ctx context.Context, k8sClient client.Client, configMapRef *v1.SecretReference) (*v1.ConfigMap, error) { + configMap := &v1.ConfigMap{} + namespacedName := client.ObjectKey{Name: configMapRef.Name, Namespace: configMapRef.Namespace} + err := k8sClient.Get(ctx, namespacedName, configMap) + if err != nil { + errMsg := "Failed to get the terraform registry config configmap" + klog.ErrorS(err, errMsg, "Name", configMapRef.Name, "Namespace", configMapRef.Namespace) + return nil, errors.Wrap(err, errMsg) + } + neededKeys := []string{TerraformRegistryConfig} + for _, key := range neededKeys { + if _, ok := configMap.Data[key]; !ok { + err := errors.Errorf("'%s' not in terraform registry configuration configmap", key) + return nil, err + } + } + + return configMap, nil +} + +// GetTerraformCredentialsHelperConfigMap get the config map containing the terraform credentials helper +func GetTerraformCredentialsHelperConfigMap(ctx context.Context, k8sClient client.Client, configMapRef *v1.SecretReference) (*v1.ConfigMap, error) { + configMap := &v1.ConfigMap{} + namespacedName := client.ObjectKey{Name: configMapRef.Name, Namespace: configMapRef.Namespace} + err := k8sClient.Get(ctx, namespacedName, configMap) + if err != nil { + errMsg := "Failed to get the terraform credentials helper configmap" + klog.ErrorS(err, errMsg, "Name", configMapRef.Name, "Namespace", configMapRef.Namespace) + return nil, errors.Wrap(err, errMsg) + } + + return configMap, nil +} diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 9aa5c221..71d38710 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1639,7 +1639,7 @@ func TestAssembleTerraformJobWithGitCredentialsSecretRef(t *testing.T) { assert.Contains(t, spec.Volumes, gitAuthSecretVolume) } -func TestAssembleTerraformJobWithTerraformCredentialsSecretRef(t *testing.T) { +func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsSecretRef(t *testing.T) { meta := &TFConfigurationMeta{ Name: "a", ConfigurationCMName: "b", @@ -1648,9 +1648,9 @@ func TestAssembleTerraformJobWithTerraformCredentialsSecretRef(t *testing.T) { Namespace: "e", TerraformImage: "f", RemoteGit: "g", - GitCredentialsSecretReference: &corev1.SecretReference{ + TerraformRegistryConfigMapReference: &corev1.SecretReference{ Namespace: "default", - Name: "git-ssh", + Name: "terraform-registry-config", }, TerraformCredentialsSecretReference: &corev1.SecretReference{ Namespace: "default", @@ -1662,6 +1662,19 @@ func TestAssembleTerraformJobWithTerraformCredentialsSecretRef(t *testing.T) { spec := job.Spec.Template.Spec var terraformSecretDefaultMode int32 = 0400 + + terraformRegistryConfigMapVolume := corev1.Volume{Name: TerraformRegistryConfigVolumeName} + terraformRegistryConfigMapVolume.ConfigMap = &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "terraform-registry-config", + }, + DefaultMode: &terraformSecretDefaultMode, + } + + terraformRegistryConfigVolumeMount := corev1.VolumeMount{ + Name: TerraformRegistryConfigVolumeName, + MountPath: TerraformRegistryConfigVolumeMountPath, + } terraformCredentialsSecretVolume := corev1.Volume{Name: TerraformCredentialsConfigVolumeName} terraformCredentialsSecretVolume.Secret = &corev1.SecretVolumeSource{ SecretName: "terraform-credentials", @@ -1673,11 +1686,71 @@ func TestAssembleTerraformJobWithTerraformCredentialsSecretRef(t *testing.T) { MountPath: TerraformCredentialsConfigVolumeMountPath, } - assert.Contains(t, spec.InitContainers[2].VolumeMounts, terraformCredentialsSecretVolumeMount) + assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformRegistryConfigVolumeMount) + assert.Contains(t, spec.Volumes, terraformRegistryConfigMapVolume) + + assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformCredentialsSecretVolumeMount) assert.Contains(t, spec.Volumes, terraformCredentialsSecretVolume) } +func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsHelperConfigMapRef(t *testing.T) { + meta := &TFConfigurationMeta{ + Name: "a", + ConfigurationCMName: "b", + BusyboxImage: "c", + GitImage: "d", + Namespace: "e", + TerraformImage: "f", + RemoteGit: "g", + TerraformRegistryConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-registry-config", + }, + TerraformCredentialsHelperConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-credentials-helper", + }, + } + + job := meta.assembleTerraformJob(TerraformApply) + spec := job.Spec.Template.Spec + + var terraformSecretDefaultMode int32 = 0400 + + terraformRegistryConfigMapVolume := corev1.Volume{Name: TerraformRegistryConfigVolumeName} + terraformRegistryConfigMapVolume.ConfigMap = &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "terraform-registry-config", + }, + DefaultMode: &terraformSecretDefaultMode, + } + + terraformRegistryConfigVolumeMount := corev1.VolumeMount{ + Name: TerraformRegistryConfigVolumeName, + MountPath: TerraformRegistryConfigVolumeMountPath, + } + terraformCredentialsHelperConfigVolume := corev1.Volume{Name: TerraformCredentialsHelperConfigVolumeName} + terraformCredentialsHelperConfigVolume.ConfigMap = &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "terraform-credentials-helper", + }, + DefaultMode: &terraformSecretDefaultMode, + } + + terraformCredentialsHelperConfigVolumeMount := corev1.VolumeMount{ + Name: TerraformCredentialsHelperConfigVolumeName, + MountPath: TerraformCredentialsHelperConfigVolumeMountPath, + } + + assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformRegistryConfigVolumeMount) + assert.Contains(t, spec.Volumes, terraformRegistryConfigMapVolume) + + assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformCredentialsHelperConfigVolumeMount) + assert.Contains(t, spec.Volumes, terraformCredentialsHelperConfigVolume) + +} + func TestTfStatePropertyToToProperty(t *testing.T) { testcases := []TfStateProperty{ { @@ -2734,19 +2807,6 @@ func TestCheckTerraformCredentialsSecretReference(t *testing.T) { assert.Nil(t, k8sClient.Create(ctx, secretNotTerraformCreds)) - secretNotTerraformRc := &corev1.Secret{ - ObjectMeta: v1.ObjectMeta{ - Namespace: "default", - Name: "terraform-creds-no-terraformrc", - }, - Data: map[string][]byte{ - "credentials.tfrc.json": credentialstfrcjson, - }, - Type: corev1.SecretTypeSSHAuth, - } - - assert.Nil(t, k8sClient.Create(ctx, secretNotTerraformRc)) - type args struct { k8sClient client.Client TerraformCredentialsSecretReference *corev1.SecretReference @@ -2775,19 +2835,6 @@ func TestCheckTerraformCredentialsSecretReference(t *testing.T) { errMsg: "Failed to get terraform credentials secret: secrets \"terraform-credentials\" not found", }, }, - { - name: "key 'terraformrc' not in terraform credentials secret", - args: args{ - k8sClient: k8sClient, - TerraformCredentialsSecretReference: &corev1.SecretReference{ - Namespace: "default", - Name: "terraform-creds-no-terraformrc", - }, - }, - want: want{ - errMsg: fmt.Sprintf("'%s' not in terraform credentials secret", TerraformRegistry), - }, - }, { name: "key 'credentials.tfrc.json' not in terraform credentials secret", args: args{ @@ -2830,3 +2877,181 @@ func TestCheckTerraformCredentialsSecretReference(t *testing.T) { } } + +func TestCheckTerraformRegistryConfigMapReference(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + corev1.AddToScheme(scheme) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + configMap := &corev1.ConfigMap{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Name: "terraform-registry-config", + }, + Data: map[string]string{ + ".terraformrc": "tfrc", + }, + } + + assert.Nil(t, k8sClient.Create(ctx, configMap)) + assert.Nil(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(configMap), configMap)) + + configMapNotTerraformRc := &corev1.ConfigMap{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Name: "terraform-registry-config-no-terraformrc", + }, + Data: map[string]string{ + "terraform": "tfrc", + }, + } + + assert.Nil(t, k8sClient.Create(ctx, configMapNotTerraformRc)) + assert.Nil(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(configMapNotTerraformRc), configMapNotTerraformRc)) + + type args struct { + k8sClient client.Client + TerraformRegistryConfigMapReference *corev1.SecretReference + } + + type want struct { + configMap *corev1.ConfigMap + errMsg string + } + + testcases := []struct { + name string + args args + want want + }{ + { + name: "configmap not found", + args: args{ + k8sClient: k8sClient, + TerraformRegistryConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-registry", + }, + }, + want: want{ + errMsg: "Failed to get the terraform registry config configmap: configmaps \"terraform-registry\" not found", + }, + }, + { + name: "key '.terraformrc' not in terraform registry config", + args: args{ + k8sClient: k8sClient, + TerraformRegistryConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-registry-config-no-terraformrc", + }, + }, + want: want{ + errMsg: fmt.Sprintf("'%s' not in terraform registry configuration configmap", TerraformRegistryConfig), + }, + }, + { + name: "configmap exists", + args: args{ + k8sClient: k8sClient, + TerraformRegistryConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-registry-config", + }, + }, + want: want{ + configMap: configMap, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + configMap, err := GetTerraformRegistryConfigMap(ctx, tc.args.k8sClient, tc.args.TerraformRegistryConfigMapReference) + + if err != nil { + assert.EqualError(t, err, tc.want.errMsg) + } + if tc.want.configMap != nil { + assert.EqualValues(t, configMap, tc.want.configMap) + } + }) + } +} + +func TestTerraformCredentialsHelperConfigMap(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + corev1.AddToScheme(scheme) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + configMap := &corev1.ConfigMap{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Name: "terraform-credentials-helper", + }, + Data: map[string]string{ + "terraform-credentials-artifactory": "tfrc", + }, + } + + assert.Nil(t, k8sClient.Create(ctx, configMap)) + assert.Nil(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(configMap), configMap)) + + type args struct { + k8sClient client.Client + TerraformCredentialsHelperConfigMapReference *corev1.SecretReference + } + + type want struct { + configMap *corev1.ConfigMap + errMsg string + } + + testcases := []struct { + name string + args args + want want + }{ + { + name: "configmap not found", + args: args{ + k8sClient: k8sClient, + TerraformCredentialsHelperConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-registry", + }, + }, + want: want{ + errMsg: "Failed to get the terraform credentials helper configmap: configmaps \"terraform-registry\" not found", + }, + }, + { + name: "configmap exists", + args: args{ + k8sClient: k8sClient, + TerraformCredentialsHelperConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-credentials-helper", + }, + }, + want: want{ + configMap: configMap, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + configMap, err := GetTerraformCredentialsHelperConfigMap(ctx, tc.args.k8sClient, tc.args.TerraformCredentialsHelperConfigMapReference) + + if err != nil { + assert.EqualError(t, err, tc.want.errMsg) + } + if tc.want.configMap != nil { + assert.EqualValues(t, configMap, tc.want.configMap) + } + }) + } +} From 55788118a71f9b00595c405d46f132e5f13352d4 Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Fri, 13 Jan 2023 14:51:47 -0500 Subject: [PATCH 07/15] feat: updated variable name to TerraformRC instead of TerraformRegistry as per review commments. Signed-off-by: raradhakrishnan --- api/types/state.go | 2 +- api/v1beta1/configuration_types.go | 4 ++-- api/v1beta1/zz_generated.deepcopy.go | 4 ++-- api/v1beta2/configuration_types.go | 4 ++-- api/v1beta2/zz_generated.deepcopy.go | 4 ++-- ...terraform.core.oam.dev_configurations.yaml | 8 ++++---- controllers/configuration_controller.go | 20 +++++++++---------- controllers/configuration_controller_test.go | 18 ++++++++--------- 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/api/types/state.go b/api/types/state.go index 4adea760..925e237c 100644 --- a/api/types/state.go +++ b/api/types/state.go @@ -36,7 +36,7 @@ const ( TerraformInitError ConfigurationState = "TerraformInitError" InvalidGitCredentialsSecretReference ConfigurationState = "InvalidGitCredentialsSecretReference" InvalidTerraformCredentialsSecretReference ConfigurationState = "InvalidTerraformCredentialsSecretReference" - InvalidTerraformRegistryConfigMapReference ConfigurationState = "InvalidTerraformRegistryConfigMapReference" + InvalidTerraformRCConfigMapReference ConfigurationState = "InvalidTerraformRCConfigMapReference" InvalidTerraformCredentialsHelperConfigMapReference ConfigurationState = "InvalidTerraformCredentialsHelperConfigMapReference" ) diff --git a/api/v1beta1/configuration_types.go b/api/v1beta1/configuration_types.go index 6e2cd5ce..1f2adb0b 100644 --- a/api/v1beta1/configuration_types.go +++ b/api/v1beta1/configuration_types.go @@ -55,8 +55,8 @@ type ConfigurationSpec struct { // TerraformCredentialsSecretReference specifies the reference to the secret containing the terraform credentials TerraformCredentialsSecretReference *v1.SecretReference `json:"terraformCredentialsSecretReference,omitempty"` - // TerraformRegistryConfigMapReference specifies the reference to a config map containing the terraform registry configuration - TerraformRegistryConfigMapReference *v1.SecretReference `json:"terraformRegistryConfigMapReference,omitempty"` + // TerraformRCConfigMapReference specifies the reference to a config map containing the terraform registry configuration + TerraformRCConfigMapReference *v1.SecretReference `json:"terraformRCConfigMapReference,omitempty"` // TerraformCredentialsHelperConfigMapReference specifies the reference to a configmap containing the terraform registry credentials helper TerraformCredentialsHelperConfigMapReference *v1.SecretReference `json:"terraformCredentialsHelperConfigMapReference,omitempty"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 3ae47a08..454b670f 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -187,8 +187,8 @@ func (in *ConfigurationSpec) DeepCopyInto(out *ConfigurationSpec) { *out = new(v1.SecretReference) **out = **in } - if in.TerraformRegistryConfigMapReference != nil { - in, out := &in.TerraformRegistryConfigMapReference, &out.TerraformRegistryConfigMapReference + if in.TerraformRCConfigMapReference != nil { + in, out := &in.TerraformRCConfigMapReference, &out.TerraformRCConfigMapReference *out = new(v1.SecretReference) **out = **in } diff --git a/api/v1beta2/configuration_types.go b/api/v1beta2/configuration_types.go index d7ff2763..8cee4e68 100644 --- a/api/v1beta2/configuration_types.go +++ b/api/v1beta2/configuration_types.go @@ -83,8 +83,8 @@ type ConfigurationSpec struct { // TerraformCredentialsSecretReference specifies the reference to the secret containing the terraform credentials and terraform registry details TerraformCredentialsSecretReference *v1.SecretReference `json:"terraformCredentialsSecretReference,omitempty"` - // TerraformRegistryConfigMapReference specifies the reference to a config map containing the terraform registry configuration - TerraformRegistryConfigMapReference *v1.SecretReference `json:"terraformRegistryConfigMapReference,omitempty"` + // TerraformRCConfigMapReference specifies the reference to a config map containing the terraform registry configuration + TerraformRCConfigMapReference *v1.SecretReference `json:"terraformRCConfigMapReference,omitempty"` // TerraformCredentialsHelperConfigMapReference specifies the reference to a configmap containing the terraform registry credentials helper TerraformCredentialsHelperConfigMapReference *v1.SecretReference `json:"terraformCredentialsHelperConfigMapReference,omitempty"` diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 0904cb8f..56fadfce 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -196,8 +196,8 @@ func (in *ConfigurationSpec) DeepCopyInto(out *ConfigurationSpec) { *out = new(v1.SecretReference) **out = **in } - if in.TerraformRegistryConfigMapReference != nil { - in, out := &in.TerraformRegistryConfigMapReference, &out.TerraformRegistryConfigMapReference + if in.TerraformRCConfigMapReference != nil { + in, out := &in.TerraformRCConfigMapReference, &out.TerraformRCConfigMapReference *out = new(v1.SecretReference) **out = **in } diff --git a/chart/crds/terraform.core.oam.dev_configurations.yaml b/chart/crds/terraform.core.oam.dev_configurations.yaml index c5cacec3..ae35b5d9 100644 --- a/chart/crds/terraform.core.oam.dev_configurations.yaml +++ b/chart/crds/terraform.core.oam.dev_configurations.yaml @@ -138,8 +138,8 @@ spec: name must be unique. type: string type: object - terraformRegistryConfigMapReference: - description: TerraformRegistryConfigMapReference specifies the reference + terraformRCConfigMapReference: + description: TerraformRCConfigMapReference specifies the reference to a config map containing the terraform registry configuration properties: name: @@ -386,8 +386,8 @@ spec: name must be unique. type: string type: object - terraformRegistryConfigMapReference: - description: TerraformRegistryConfigMapReference specifies the reference + terraformRCConfigMapReference: + description: TerraformRCConfigMapReference specifies the reference to a config map containing the terraform registry configuration properties: name: diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index b58f2df9..8f2d246c 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -274,7 +274,7 @@ type TFConfigurationMeta struct { JobEnv map[string]interface{} GitCredentialsSecretReference *v1.SecretReference TerraformCredentialsSecretReference *v1.SecretReference - TerraformRegistryConfigMapReference *v1.SecretReference + TerraformRCConfigMapReference *v1.SecretReference TerraformCredentialsHelperConfigMapReference *v1.SecretReference Backend backend.Backend @@ -345,8 +345,8 @@ func initTFConfigurationMeta(req ctrl.Request, configuration v1beta2.Configurati meta.TerraformCredentialsSecretReference = configuration.Spec.TerraformCredentialsSecretReference } - if configuration.Spec.TerraformRegistryConfigMapReference != nil { - meta.TerraformRegistryConfigMapReference = configuration.Spec.TerraformRegistryConfigMapReference + if configuration.Spec.TerraformRCConfigMapReference != nil { + meta.TerraformRCConfigMapReference = configuration.Spec.TerraformRCConfigMapReference } if configuration.Spec.TerraformCredentialsHelperConfigMapReference != nil { @@ -616,14 +616,14 @@ func (r *ConfigurationReconciler) preCheck(ctx context.Context, configuration *v } } - if meta.TerraformRegistryConfigMapReference != nil { - terraformRegistryConfig, err := GetTerraformRegistryConfigMap(ctx, k8sClient, meta.TerraformRegistryConfigMapReference) + if meta.TerraformRCConfigMapReference != nil { + terraformRegistryConfig, err := GetTerraformRegistryConfigMap(ctx, k8sClient, meta.TerraformRCConfigMapReference) if terraformRegistryConfig == nil { - msg := string(types.InvalidTerraformRegistryConfigMapReference) + msg := string(types.InvalidTerraformRCConfigMapReference) if err != nil { msg = err.Error() } - if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.InvalidTerraformRegistryConfigMapReference, msg); updateStatusErr != nil { + if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.InvalidTerraformRCConfigMapReference, msg); updateStatusErr != nil { return errors.Wrap(updateStatusErr, msg) } return errors.New(msg) @@ -819,7 +819,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe }) } - if meta.TerraformRegistryConfigMapReference != nil { + if meta.TerraformRCConfigMapReference != nil { initContainerVolumeMounts = append(initContainerVolumeMounts, v1.VolumeMount{ Name: TerraformRegistryConfigVolumeName, @@ -1002,7 +1002,7 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume { terraformCredentialsConfigVolume := meta.createTerraformCredentialsConfigVolume() executorVolumes = append(executorVolumes, terraformCredentialsConfigVolume) } - if meta.TerraformRegistryConfigMapReference != nil { + if meta.TerraformRCConfigMapReference != nil { terraformRegistryConfigVolume := meta.createTerraformRegistryConfigVolume() executorVolumes = append(executorVolumes, terraformRegistryConfigVolume) } @@ -1051,7 +1051,7 @@ func (meta *TFConfigurationMeta) createTerraformCredentialsConfigVolume() v1.Vol func (meta *TFConfigurationMeta) createTerraformRegistryConfigVolume() v1.Volume { var terraformConfigMapDefaultMode int32 = 0400 terraformRegistryConfigMapVolumeSource := v1.ConfigMapVolumeSource{} - terraformRegistryConfigMapVolumeSource.Name = meta.TerraformRegistryConfigMapReference.Name + terraformRegistryConfigMapVolumeSource.Name = meta.TerraformRCConfigMapReference.Name terraformRegistryConfigMapVolumeSource.DefaultMode = &terraformConfigMapDefaultMode terraformRegistryConfigMapVolume := v1.Volume{Name: TerraformRegistryConfigVolumeName} terraformRegistryConfigMapVolume.ConfigMap = &terraformRegistryConfigMapVolumeSource diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 71d38710..5cf66412 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1648,7 +1648,7 @@ func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsSecretRef( Namespace: "e", TerraformImage: "f", RemoteGit: "g", - TerraformRegistryConfigMapReference: &corev1.SecretReference{ + TerraformRCConfigMapReference: &corev1.SecretReference{ Namespace: "default", Name: "terraform-registry-config", }, @@ -1703,7 +1703,7 @@ func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsHelperConf Namespace: "e", TerraformImage: "f", RemoteGit: "g", - TerraformRegistryConfigMapReference: &corev1.SecretReference{ + TerraformRCConfigMapReference: &corev1.SecretReference{ Namespace: "default", Name: "terraform-registry-config", }, @@ -2878,7 +2878,7 @@ func TestCheckTerraformCredentialsSecretReference(t *testing.T) { } -func TestCheckTerraformRegistryConfigMapReference(t *testing.T) { +func TestCheckTerraformRCConfigMapReference(t *testing.T) { ctx := context.Background() scheme := runtime.NewScheme() corev1.AddToScheme(scheme) @@ -2911,8 +2911,8 @@ func TestCheckTerraformRegistryConfigMapReference(t *testing.T) { assert.Nil(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(configMapNotTerraformRc), configMapNotTerraformRc)) type args struct { - k8sClient client.Client - TerraformRegistryConfigMapReference *corev1.SecretReference + k8sClient client.Client + TerraformRCConfigMapReference *corev1.SecretReference } type want struct { @@ -2929,7 +2929,7 @@ func TestCheckTerraformRegistryConfigMapReference(t *testing.T) { name: "configmap not found", args: args{ k8sClient: k8sClient, - TerraformRegistryConfigMapReference: &corev1.SecretReference{ + TerraformRCConfigMapReference: &corev1.SecretReference{ Namespace: "default", Name: "terraform-registry", }, @@ -2942,7 +2942,7 @@ func TestCheckTerraformRegistryConfigMapReference(t *testing.T) { name: "key '.terraformrc' not in terraform registry config", args: args{ k8sClient: k8sClient, - TerraformRegistryConfigMapReference: &corev1.SecretReference{ + TerraformRCConfigMapReference: &corev1.SecretReference{ Namespace: "default", Name: "terraform-registry-config-no-terraformrc", }, @@ -2955,7 +2955,7 @@ func TestCheckTerraformRegistryConfigMapReference(t *testing.T) { name: "configmap exists", args: args{ k8sClient: k8sClient, - TerraformRegistryConfigMapReference: &corev1.SecretReference{ + TerraformRCConfigMapReference: &corev1.SecretReference{ Namespace: "default", Name: "terraform-registry-config", }, @@ -2968,7 +2968,7 @@ func TestCheckTerraformRegistryConfigMapReference(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - configMap, err := GetTerraformRegistryConfigMap(ctx, tc.args.k8sClient, tc.args.TerraformRegistryConfigMapReference) + configMap, err := GetTerraformRegistryConfigMap(ctx, tc.args.k8sClient, tc.args.TerraformRCConfigMapReference) if err != nil { assert.EqualError(t, err, tc.want.errMsg) From fcb253764c272caa6bfd994e5e47efeda307b29f Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Fri, 13 Jan 2023 15:44:12 -0500 Subject: [PATCH 08/15] feat: unified similar functions to create a volume with either secret or configMap. Review comment fixed. Signed-off-by: raradhakrishnan --- controllers/configuration_controller.go | 73 +++++++------------- controllers/configuration_controller_test.go | 12 ++-- 2 files changed, 31 insertions(+), 54 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 8f2d246c..e0dd7155 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -73,10 +73,10 @@ const ( TerraformCredentialsConfigVolumeName = "terraform-credentials-configuration" // TerraformCredentialsConfigVolumeMountPath is the volume mount path for terraform auth configurtaion TerraformCredentialsConfigVolumeMountPath = "/root/.terraform.d" - // TerraformRegistryConfigVolumeName is the volume name of the terraform registry configuration - TerraformRegistryConfigVolumeName = "terraform-registry-configuration" - // TerraformRegistryConfigVolumeMountPath is the volume mount path for registry configuration - TerraformRegistryConfigVolumeMountPath = "/root" + // TerraformRCConfigVolumeName is the volume name of the terraform registry configuration + TerraformRCConfigVolumeName = "terraform-rc-configuration" + // TerraformRCConfigVolumeMountPath is the volume mount path for registry configuration + TerraformRCConfigVolumeMountPath = "/root" // TerraformCredentialsHelperConfigVolumeName is the volume name for terraform auth configurtaion TerraformCredentialsHelperConfigVolumeName = "terraform-credentials-helper-configuration" // TerraformCredentialsHelperConfigVolumeMountPath is the volume mount path for terraform auth configurtaion @@ -822,8 +822,8 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe if meta.TerraformRCConfigMapReference != nil { initContainerVolumeMounts = append(initContainerVolumeMounts, v1.VolumeMount{ - Name: TerraformRegistryConfigVolumeName, - MountPath: TerraformRegistryConfigVolumeMountPath, + Name: TerraformRCConfigVolumeName, + MountPath: TerraformRCConfigVolumeMountPath, }) } @@ -995,19 +995,19 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume { tfBackendVolume := meta.createTFBackendVolume() executorVolumes := []v1.Volume{workingVolume, inputTFConfigurationVolume, tfBackendVolume} if meta.GitCredentialsSecretReference != nil { - gitAuthConfigVolume := meta.createGitAuthConfigVolume() + gitAuthConfigVolume := meta.createSecretOrConfigMapVolume(true, meta.GitCredentialsSecretReference.Name, GitAuthConfigVolumeName) executorVolumes = append(executorVolumes, gitAuthConfigVolume) } if meta.TerraformCredentialsSecretReference != nil { - terraformCredentialsConfigVolume := meta.createTerraformCredentialsConfigVolume() + terraformCredentialsConfigVolume := meta.createSecretOrConfigMapVolume(true, meta.TerraformCredentialsSecretReference.Name, TerraformCredentialsConfigVolumeName) executorVolumes = append(executorVolumes, terraformCredentialsConfigVolume) } if meta.TerraformRCConfigMapReference != nil { - terraformRegistryConfigVolume := meta.createTerraformRegistryConfigVolume() + terraformRegistryConfigVolume := meta.createSecretOrConfigMapVolume(false, meta.TerraformRCConfigMapReference.Name, TerraformRCConfigVolumeName) executorVolumes = append(executorVolumes, terraformRegistryConfigVolume) } if meta.TerraformCredentialsHelperConfigMapReference != nil { - terraformCredentialsHelperConfigVolume := meta.createTerraformCredentialsHelperConfigVolume() + terraformCredentialsHelperConfigVolume := meta.createSecretOrConfigMapVolume(false, meta.TerraformCredentialsHelperConfigMapReference.Name, TerraformCredentialsHelperConfigVolumeName) executorVolumes = append(executorVolumes, terraformCredentialsHelperConfigVolume) } return executorVolumes @@ -1028,44 +1028,21 @@ func (meta *TFConfigurationMeta) createTFBackendVolume() v1.Volume { return gitVolume } -func (meta *TFConfigurationMeta) createGitAuthConfigVolume() v1.Volume { - var gitSecretDefaultMode int32 = 0400 - gitAuthSecretVolumeSource := v1.SecretVolumeSource{} - gitAuthSecretVolumeSource.SecretName = meta.GitCredentialsSecretReference.Name - gitAuthSecretVolumeSource.DefaultMode = &gitSecretDefaultMode - gitAuthSecretVolume := v1.Volume{Name: GitAuthConfigVolumeName} - gitAuthSecretVolume.Secret = &gitAuthSecretVolumeSource - return gitAuthSecretVolume -} - -func (meta *TFConfigurationMeta) createTerraformCredentialsConfigVolume() v1.Volume { - var terraformSecretDefaultMode int32 = 0400 - terraformCredentialsSecretVolumeSource := v1.SecretVolumeSource{} - terraformCredentialsSecretVolumeSource.SecretName = meta.TerraformCredentialsSecretReference.Name - terraformCredentialsSecretVolumeSource.DefaultMode = &terraformSecretDefaultMode - terraformCredentialsSecretVolume := v1.Volume{Name: TerraformCredentialsConfigVolumeName} - terraformCredentialsSecretVolume.Secret = &terraformCredentialsSecretVolumeSource - return terraformCredentialsSecretVolume -} - -func (meta *TFConfigurationMeta) createTerraformRegistryConfigVolume() v1.Volume { - var terraformConfigMapDefaultMode int32 = 0400 - terraformRegistryConfigMapVolumeSource := v1.ConfigMapVolumeSource{} - terraformRegistryConfigMapVolumeSource.Name = meta.TerraformRCConfigMapReference.Name - terraformRegistryConfigMapVolumeSource.DefaultMode = &terraformConfigMapDefaultMode - terraformRegistryConfigMapVolume := v1.Volume{Name: TerraformRegistryConfigVolumeName} - terraformRegistryConfigMapVolume.ConfigMap = &terraformRegistryConfigMapVolumeSource - return terraformRegistryConfigMapVolume -} - -func (meta *TFConfigurationMeta) createTerraformCredentialsHelperConfigVolume() v1.Volume { - var terraformConfigMapDefaultMode int32 = 0400 - terraformCredentialsHelperConfigMapVolumeSource := v1.ConfigMapVolumeSource{} - terraformCredentialsHelperConfigMapVolumeSource.Name = meta.TerraformCredentialsHelperConfigMapReference.Name - terraformCredentialsHelperConfigMapVolumeSource.DefaultMode = &terraformConfigMapDefaultMode - terraformCredentialsHelperConfigMapVolume := v1.Volume{Name: TerraformCredentialsHelperConfigVolumeName} - terraformCredentialsHelperConfigMapVolume.ConfigMap = &terraformCredentialsHelperConfigMapVolumeSource - return terraformCredentialsHelperConfigMapVolume +func (meta *TFConfigurationMeta) createSecretOrConfigMapVolume(isSecret bool, secretOrConfigMapReferenceName string, volumeName string) v1.Volume { + var defaultMode int32 = 0400 + volume := v1.Volume{Name: volumeName} + if isSecret { + volumeSource := v1.SecretVolumeSource{} + volumeSource.SecretName = secretOrConfigMapReferenceName + volumeSource.DefaultMode = &defaultMode + volume.Secret = &volumeSource + } else { + volumeSource := v1.ConfigMapVolumeSource{} + volumeSource.Name = secretOrConfigMapReferenceName + volumeSource.DefaultMode = &defaultMode + volume.ConfigMap = &volumeSource + } + return volume } // TfStateProperty is the tf state property for an output diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 5cf66412..b4d1cf88 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1663,7 +1663,7 @@ func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsSecretRef( var terraformSecretDefaultMode int32 = 0400 - terraformRegistryConfigMapVolume := corev1.Volume{Name: TerraformRegistryConfigVolumeName} + terraformRegistryConfigMapVolume := corev1.Volume{Name: TerraformRCConfigVolumeName} terraformRegistryConfigMapVolume.ConfigMap = &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ Name: "terraform-registry-config", @@ -1672,8 +1672,8 @@ func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsSecretRef( } terraformRegistryConfigVolumeMount := corev1.VolumeMount{ - Name: TerraformRegistryConfigVolumeName, - MountPath: TerraformRegistryConfigVolumeMountPath, + Name: TerraformRCConfigVolumeName, + MountPath: TerraformRCConfigVolumeMountPath, } terraformCredentialsSecretVolume := corev1.Volume{Name: TerraformCredentialsConfigVolumeName} terraformCredentialsSecretVolume.Secret = &corev1.SecretVolumeSource{ @@ -1718,7 +1718,7 @@ func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsHelperConf var terraformSecretDefaultMode int32 = 0400 - terraformRegistryConfigMapVolume := corev1.Volume{Name: TerraformRegistryConfigVolumeName} + terraformRegistryConfigMapVolume := corev1.Volume{Name: TerraformRCConfigVolumeName} terraformRegistryConfigMapVolume.ConfigMap = &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ Name: "terraform-registry-config", @@ -1727,8 +1727,8 @@ func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsHelperConf } terraformRegistryConfigVolumeMount := corev1.VolumeMount{ - Name: TerraformRegistryConfigVolumeName, - MountPath: TerraformRegistryConfigVolumeMountPath, + Name: TerraformRCConfigVolumeName, + MountPath: TerraformRCConfigVolumeMountPath, } terraformCredentialsHelperConfigVolume := corev1.Volume{Name: TerraformCredentialsHelperConfigVolumeName} terraformCredentialsHelperConfigVolume.ConfigMap = &corev1.ConfigMapVolumeSource{ From fbf828ba18894a710e106d4782627673afbfb82e Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Fri, 13 Jan 2023 17:31:42 -0500 Subject: [PATCH 09/15] feat: GetFooSecret/GetFooConfigMap unified with a generator function.fixed review comment. Signed-off-by: raradhakrishnan --- controllers/configuration_controller.go | 122 ++++++++++--------- controllers/configuration_controller_test.go | 1 - 2 files changed, 66 insertions(+), 57 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index e0dd7155..193dc0b8 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -1454,81 +1454,91 @@ func (meta *TFConfigurationMeta) RenderConfiguration(configuration *v1beta2.Conf // GetGitCredentialsSecret will get the secret containing the SSH private key & known_hosts func GetGitCredentialsSecret(ctx context.Context, k8sClient client.Client, secretRef *v1.SecretReference) (*v1.Secret, error) { - secret := &v1.Secret{} - - namespacedName := client.ObjectKey{Name: secretRef.Name, Namespace: secretRef.Namespace} - err := k8sClient.Get(ctx, namespacedName, secret) - if err != nil { - errMsg := "Failed to get git credentials secret" - klog.ErrorS(err, errMsg, "Name", secretRef.Name, "Namespace", secretRef.Namespace) - return nil, errors.Wrap(err, errMsg) - } - needSecretKeys := []string{GitCredsKnownHosts, v1.SSHAuthPrivateKey} - for _, key := range needSecretKeys { - if _, ok := secret.Data[key]; !ok { - err := errors.Errorf("'%s' not in git credentials secret", key) - return nil, err - } + errMsg := "Failed to get git credentials secret" + keyErrMsg := "not in git credentials secret" + secret, _, err := GetSecretOrConfigMap(ctx, k8sClient, true, secretRef, needSecretKeys, errMsg, keyErrMsg) + if secret != nil { + return secret, nil + } else { + return nil, err } - - return secret, nil } // GetTerraformCredentialsSecret will get the secret containing the terraform credentials func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, secretRef *v1.SecretReference) (*v1.Secret, error) { - secret := &v1.Secret{} - namespacedName := client.ObjectKey{Name: secretRef.Name, Namespace: secretRef.Namespace} - err := k8sClient.Get(ctx, namespacedName, secret) - if err != nil { - errMsg := "Failed to get terraform credentials secret" - klog.ErrorS(err, errMsg, "Name", secretRef.Name, "Namespace", secretRef.Namespace) - return nil, errors.Wrap(err, errMsg) - } - needSecretKeys := []string{TerraformCredentials} - for _, key := range needSecretKeys { - if _, ok := secret.Data[key]; !ok { - err := errors.Errorf("'%s' not in terraform credentials secret", key) - return nil, err - } + errMsg := "Failed to get terraform credentials secret" + keyErrMsg := "not in terraform credentials secret" + secret, _, err := GetSecretOrConfigMap(ctx, k8sClient, true, secretRef, needSecretKeys, errMsg, keyErrMsg) + if secret != nil { + return secret, nil + } else { + return nil, err } - - return secret, nil - } // GetTerraformRegistryConfigMap will get the config map containing the terraform registry configuration func GetTerraformRegistryConfigMap(ctx context.Context, k8sClient client.Client, configMapRef *v1.SecretReference) (*v1.ConfigMap, error) { - configMap := &v1.ConfigMap{} - namespacedName := client.ObjectKey{Name: configMapRef.Name, Namespace: configMapRef.Namespace} - err := k8sClient.Get(ctx, namespacedName, configMap) - if err != nil { - errMsg := "Failed to get the terraform registry config configmap" - klog.ErrorS(err, errMsg, "Name", configMapRef.Name, "Namespace", configMapRef.Namespace) - return nil, errors.Wrap(err, errMsg) - } neededKeys := []string{TerraformRegistryConfig} - for _, key := range neededKeys { - if _, ok := configMap.Data[key]; !ok { - err := errors.Errorf("'%s' not in terraform registry configuration configmap", key) - return nil, err - } + errMsg := "Failed to get the terraform registry config configmap" + keyErrMsg := "not in terraform registry configuration configmap" + _, configMap, err := GetSecretOrConfigMap(ctx, k8sClient, false, configMapRef, neededKeys, errMsg, keyErrMsg) + if configMap != nil { + return configMap, nil + } else { + return nil, err } - - return configMap, nil } // GetTerraformCredentialsHelperConfigMap get the config map containing the terraform credentials helper func GetTerraformCredentialsHelperConfigMap(ctx context.Context, k8sClient client.Client, configMapRef *v1.SecretReference) (*v1.ConfigMap, error) { + errMsg := "Failed to get the terraform credentials helper configmap" + neededKeys := []string{} + _, configMap, err := GetSecretOrConfigMap(ctx, k8sClient, false, configMapRef, neededKeys, errMsg, "") + if configMap != nil { + return configMap, nil + } else { + return nil, err + } +} + +func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret bool, ref *v1.SecretReference, neededKeys []string, errMsg string, keyErrMsg string) (*v1.Secret, *v1.ConfigMap, error) { + secret := &v1.Secret{} configMap := &v1.ConfigMap{} - namespacedName := client.ObjectKey{Name: configMapRef.Name, Namespace: configMapRef.Namespace} - err := k8sClient.Get(ctx, namespacedName, configMap) + var err error + if isSecret { + namespacedName := client.ObjectKey{Name: ref.Name, Namespace: ref.Namespace} + err = k8sClient.Get(ctx, namespacedName, secret) + } else { + namespacedName := client.ObjectKey{Name: ref.Name, Namespace: ref.Namespace} + err = k8sClient.Get(ctx, namespacedName, configMap) + } if err != nil { - errMsg := "Failed to get the terraform credentials helper configmap" - klog.ErrorS(err, errMsg, "Name", configMapRef.Name, "Namespace", configMapRef.Namespace) - return nil, errors.Wrap(err, errMsg) + klog.ErrorS(err, errMsg, "Name", ref.Name, "Namespace", ref.Namespace) + return nil, nil, errors.Wrap(err, errMsg) + } + if len(neededKeys) > 0 { + for _, key := range neededKeys { + var keyErr bool + if isSecret { + if _, ok := secret.Data[key]; !ok { + keyErr = true + } + } else { + if _, ok := configMap.Data[key]; !ok { + keyErr = true + } + } + if keyErr { + keyErr := errors.Errorf("'%s' %s", key, keyErrMsg) + return nil, nil, keyErr + } + } + } + if isSecret { + return secret, nil, nil + } else { + return nil, configMap, nil } - - return configMap, nil } diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index b4d1cf88..125e2bdd 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -2760,7 +2760,6 @@ func TestCheckGitCredentialsSecretReference(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { sec, err := GetGitCredentialsSecret(ctx, tc.args.k8sClient, tc.args.GitCredentialsSecretReference) - if err != nil { assert.EqualError(t, err, tc.want.errMsg) } From c8efbee6024ed030ad4dbed7ca22bf6311561eca Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Mon, 16 Jan 2023 11:42:43 -0500 Subject: [PATCH 10/15] feat: reduced cylomatic complexity of precheck func and organized content for secret and configmaps check. Fixed review comments. Signed-off-by: raradhakrishnan --- controllers/configuration_controller.go | 153 +++++++++++-------- controllers/configuration_controller_test.go | 2 +- 2 files changed, 88 insertions(+), 67 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 193dc0b8..23dc2439 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -588,60 +588,10 @@ func (r *ConfigurationReconciler) preCheck(ctx context.Context, configuration *v } } - if meta.GitCredentialsSecretReference != nil { - gitCreds, err := GetGitCredentialsSecret(ctx, k8sClient, meta.GitCredentialsSecretReference) - if gitCreds == nil { - msg := string(types.InvalidGitCredentialsSecretReference) - if err != nil { - msg = err.Error() - } - if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.InvalidGitCredentialsSecretReference, msg); updateStatusErr != nil { - return errors.Wrap(updateStatusErr, msg) - } - return errors.New(msg) - } - } - - if meta.TerraformCredentialsSecretReference != nil { - terraformCreds, err := GetTerraformCredentialsSecret(ctx, k8sClient, meta.TerraformCredentialsSecretReference) - if terraformCreds == nil { - msg := string(types.InvalidTerraformCredentialsSecretReference) - if err != nil { - msg = err.Error() - } - if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.InvalidTerraformCredentialsSecretReference, msg); updateStatusErr != nil { - return errors.Wrap(updateStatusErr, msg) - } - return errors.New(msg) - } - } - - if meta.TerraformRCConfigMapReference != nil { - terraformRegistryConfig, err := GetTerraformRegistryConfigMap(ctx, k8sClient, meta.TerraformRCConfigMapReference) - if terraformRegistryConfig == nil { - msg := string(types.InvalidTerraformRCConfigMapReference) - if err != nil { - msg = err.Error() - } - if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.InvalidTerraformRCConfigMapReference, msg); updateStatusErr != nil { - return errors.Wrap(updateStatusErr, msg) - } - return errors.New(msg) - } - } - - if meta.TerraformCredentialsHelperConfigMapReference != nil { - terraformCredentialsHelper, err := GetTerraformCredentialsHelperConfigMap(ctx, k8sClient, meta.TerraformCredentialsHelperConfigMapReference) - if terraformCredentialsHelper == nil { - msg := string(types.InvalidTerraformCredentialsHelperConfigMapReference) - if err != nil { - msg = err.Error() - } - if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.InvalidTerraformCredentialsHelperConfigMapReference, msg); updateStatusErr != nil { - return errors.Wrap(updateStatusErr, msg) - } - return errors.New(msg) - } + // validate secretreferences and config maps. 1. GitCredentialsSecretReference 2. TerraformCredentialsSecretReference 3. TerraformRCConfigMapReference + // 4. TerraformCredentialsHelperConfigMapReference + if err := meta.validateSecretAndConfigMap(ctx, k8sClient); err != nil { + return err } // Render configuration with backend @@ -710,6 +660,81 @@ func (r *ConfigurationReconciler) preCheck(ctx context.Context, configuration *v return createTerraformExecutorClusterRole(ctx, k8sClient, fmt.Sprintf("%s-%s", meta.ControllerNamespace, ClusterRoleName)) } +func (meta *TFConfigurationMeta) validateSecretAndConfigMap(ctx context.Context, k8sClient client.Client) error { + + secretConfigMapToCheck := []struct { + ref *v1.SecretReference + notFoundState types.ConfigurationState + refType string + }{ + { + ref: meta.GitCredentialsSecretReference, + notFoundState: types.InvalidGitCredentialsSecretReference, + refType: "GitCredentialsSecretReference", + }, + { + ref: meta.TerraformCredentialsSecretReference, + notFoundState: types.InvalidTerraformCredentialsSecretReference, + refType: "TerraformCredentialsSecretReference", + }, + { + ref: meta.TerraformRCConfigMapReference, + notFoundState: types.InvalidTerraformRCConfigMapReference, + refType: "TerraformRCConfigMapReference", + }, + { + ref: meta.TerraformCredentialsHelperConfigMapReference, + notFoundState: types.InvalidTerraformCredentialsHelperConfigMapReference, + refType: "TerraformCredentialsHelperConfigMapReference", + }, + } + + var checkErr error + var checkErrFlag bool + for _, check := range secretConfigMapToCheck { + if check.ref != nil { + switch check.refType { + case "GitCredentialsSecretReference": + gitCreds, err := GetGitCredentialsSecret(ctx, k8sClient, check.ref) + if gitCreds == nil { + checkErr = err + checkErrFlag = true + } + case "TerraformCredentialsSecretReference": + terraformCreds, err := GetTerraformCredentialsSecret(ctx, k8sClient, check.ref) + if terraformCreds == nil { + checkErr = err + checkErrFlag = true + } + case "TerraformRCConfigMapReference": + terraformRegistryConfig, err := GetTerraformRCConfigMap(ctx, k8sClient, check.ref) + if terraformRegistryConfig == nil { + checkErr = err + checkErrFlag = true + } + case "TerraformCredentialsHelperConfigMapReference": + terraformCredentialsHelper, err := GetTerraformCredentialsHelperConfigMap(ctx, k8sClient, check.ref) + if terraformCredentialsHelper == nil { + checkErr = err + checkErrFlag = true + } + } + + if checkErrFlag { + msg := string(check.notFoundState) + if checkErr != nil { + msg = checkErr.Error() + } + if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, check.notFoundState, msg); updateStatusErr != nil { + return errors.Wrap(updateStatusErr, msg) + } + return errors.New(msg) + } + } + } + return nil +} + func (meta *TFConfigurationMeta) updateApplyStatus(ctx context.Context, k8sClient client.Client, state types.ConfigurationState, message string) error { var configuration v1beta2.Configuration if err := k8sClient.Get(ctx, client.ObjectKey{Name: meta.Name, Namespace: meta.Namespace}, &configuration); err == nil { @@ -1460,9 +1485,8 @@ func GetGitCredentialsSecret(ctx context.Context, k8sClient client.Client, secre secret, _, err := GetSecretOrConfigMap(ctx, k8sClient, true, secretRef, needSecretKeys, errMsg, keyErrMsg) if secret != nil { return secret, nil - } else { - return nil, err } + return nil, err } // GetTerraformCredentialsSecret will get the secret containing the terraform credentials @@ -1473,22 +1497,20 @@ func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, secret, _, err := GetSecretOrConfigMap(ctx, k8sClient, true, secretRef, needSecretKeys, errMsg, keyErrMsg) if secret != nil { return secret, nil - } else { - return nil, err } + return nil, err } -// GetTerraformRegistryConfigMap will get the config map containing the terraform registry configuration -func GetTerraformRegistryConfigMap(ctx context.Context, k8sClient client.Client, configMapRef *v1.SecretReference) (*v1.ConfigMap, error) { +// GetTerraformRCConfigMap will get the config map containing the terraform registry configuration +func GetTerraformRCConfigMap(ctx context.Context, k8sClient client.Client, configMapRef *v1.SecretReference) (*v1.ConfigMap, error) { neededKeys := []string{TerraformRegistryConfig} errMsg := "Failed to get the terraform registry config configmap" keyErrMsg := "not in terraform registry configuration configmap" _, configMap, err := GetSecretOrConfigMap(ctx, k8sClient, false, configMapRef, neededKeys, errMsg, keyErrMsg) if configMap != nil { return configMap, nil - } else { - return nil, err } + return nil, err } // GetTerraformCredentialsHelperConfigMap get the config map containing the terraform credentials helper @@ -1498,9 +1520,8 @@ func GetTerraformCredentialsHelperConfigMap(ctx context.Context, k8sClient clien _, configMap, err := GetSecretOrConfigMap(ctx, k8sClient, false, configMapRef, neededKeys, errMsg, "") if configMap != nil { return configMap, nil - } else { - return nil, err } + return nil, err } func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret bool, ref *v1.SecretReference, neededKeys []string, errMsg string, keyErrMsg string) (*v1.Secret, *v1.ConfigMap, error) { @@ -1538,7 +1559,7 @@ func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret } if isSecret { return secret, nil, nil - } else { - return nil, configMap, nil } + + return nil, configMap, nil } diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 125e2bdd..a76fb433 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -2967,7 +2967,7 @@ func TestCheckTerraformRCConfigMapReference(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - configMap, err := GetTerraformRegistryConfigMap(ctx, tc.args.k8sClient, tc.args.TerraformRCConfigMapReference) + configMap, err := GetTerraformRCConfigMap(ctx, tc.args.k8sClient, tc.args.TerraformRCConfigMapReference) if err != nil { assert.EqualError(t, err, tc.want.errMsg) From b3a2a4bccae809dc5478641e2e9b3e8678c40789 Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Mon, 16 Jan 2023 15:53:23 -0500 Subject: [PATCH 11/15] feat: added unit test. Signed-off-by: raradhakrishnan --- controllers/configuration_controller_test.go | 194 +++++++++++++++++++ 1 file changed, 194 insertions(+) diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index a76fb433..fbfdd47a 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -3054,3 +3054,197 @@ func TestTerraformCredentialsHelperConfigMap(t *testing.T) { }) } } + +func TestCheckValidateSecretAndConfigMap(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + corev1.AddToScheme(scheme) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + privateKey := []byte("aaa") + knownHosts := []byte("zzz") + secretGitCreds := &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Name: "git-ssh", + }, + Data: map[string][]byte{ + corev1.SSHAuthPrivateKey: privateKey, + "known_hosts": knownHosts, + }, + Type: corev1.SecretTypeSSHAuth, + } + assert.Nil(t, k8sClient.Create(ctx, secretGitCreds)) + assert.Nil(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(secretGitCreds), secretGitCreds)) + + credentialstfrcjson := []byte("tfcreds") + terraformrc := []byte("tfrc") + secretTerraformCredentials := &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Name: "terraform-creds", + }, + Data: map[string][]byte{ + "credentials.tfrc.json": credentialstfrcjson, + "terraformrc": terraformrc, + }, + Type: corev1.SecretTypeSSHAuth, + } + assert.Nil(t, k8sClient.Create(ctx, secretTerraformCredentials)) + assert.Nil(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(secretTerraformCredentials), secretTerraformCredentials)) + + configMapTerraformRC := &corev1.ConfigMap{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Name: "terraform-registry-config", + }, + Data: map[string]string{ + ".terraformrc": "tfrc", + }, + } + + assert.Nil(t, k8sClient.Create(ctx, configMapTerraformRC)) + assert.Nil(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(configMapTerraformRC), configMapTerraformRC)) + + configMapCredentialsHelper := &corev1.ConfigMap{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Name: "terraform-credentials-helper", + }, + Data: map[string]string{ + "terraform-credentials-artifactory": "tfrc", + }, + } + + assert.Nil(t, k8sClient.Create(ctx, configMapCredentialsHelper)) + assert.Nil(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(configMapCredentialsHelper), configMapCredentialsHelper)) + + type args struct { + k8sClient client.Client + meta TFConfigurationMeta + } + + type want struct { + configMap *corev1.ConfigMap + errMsg string + } + + testcases := []struct { + name string + args args + want want + }{ + { + name: "configmap not found", + args: args{ + k8sClient: k8sClient, + meta: TFConfigurationMeta{ + Name: "a", + ConfigurationCMName: "b", + BusyboxImage: "c", + GitImage: "d", + Namespace: "e", + TerraformImage: "f", + RemoteGit: "g", + GitCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "git-ssh", + }, + TerraformCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-creds", + }, + TerraformRCConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-registry-config", + }, + TerraformCredentialsHelperConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-credentials-helper", + }, + }, + }, + want: want{ + errMsg: "NoError", + }, + }, + { + name: "terraform credentials configmap not found", + args: args{ + k8sClient: k8sClient, + meta: TFConfigurationMeta{ + Name: "a", + ConfigurationCMName: "b", + BusyboxImage: "c", + GitImage: "d", + Namespace: "e", + TerraformImage: "f", + RemoteGit: "g", + GitCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "git-ssh", + }, + TerraformCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-creds", + }, + TerraformRCConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-registry-config", + }, + TerraformCredentialsHelperConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-registry", + }, + }, + }, + want: want{ + errMsg: "Failed to get the terraform credentials helper configmap: configmaps \"terraform-registry\" not found", + }, + }, + { + name: "terraformrc configmap not found", + args: args{ + k8sClient: k8sClient, + meta: TFConfigurationMeta{ + Name: "a", + ConfigurationCMName: "b", + BusyboxImage: "c", + GitImage: "d", + Namespace: "e", + TerraformImage: "f", + RemoteGit: "g", + GitCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "git-ssh", + }, + TerraformCredentialsSecretReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-creds", + }, + TerraformRCConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-registry", + }, + TerraformCredentialsHelperConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-credentials-helper", + }, + }, + }, + want: want{ + errMsg: "Failed to get the terraform registry config configmap: configmaps \"terraform-registry\" not found", + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + err := tc.args.meta.validateSecretAndConfigMap(ctx, k8sClient) + if err != nil { + assert.EqualError(t, err, tc.want.errMsg) + } + }) + } + +} From 77e583ae7c7703d39c4c196eb7237ee82bd13926 Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Tue, 17 Jan 2023 14:22:17 -0500 Subject: [PATCH 12/15] feat: fixed review comments. Signed-off-by: raradhakrishnan --- controllers/configuration_controller.go | 171 +++++++------------ controllers/configuration_controller_test.go | 23 ++- 2 files changed, 84 insertions(+), 110 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 23dc2439..ffd200e9 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -588,8 +588,11 @@ func (r *ConfigurationReconciler) preCheck(ctx context.Context, configuration *v } } - // validate secretreferences and config maps. 1. GitCredentialsSecretReference 2. TerraformCredentialsSecretReference 3. TerraformRCConfigMapReference - // 4. TerraformCredentialsHelperConfigMapReference + /* validate the follwing secretreferences and config maps. + 1. GitCredentialsSecretReference + 2. TerraformCredentialsSecretReference + 3. TerraformRCConfigMapReference + 4. TerraformCredentialsHelperConfigMapReference*/ if err := meta.validateSecretAndConfigMap(ctx, k8sClient); err != nil { return err } @@ -665,65 +668,53 @@ func (meta *TFConfigurationMeta) validateSecretAndConfigMap(ctx context.Context, secretConfigMapToCheck := []struct { ref *v1.SecretReference notFoundState types.ConfigurationState - refType string + isSecret bool + neededKeys []string + errMsg string + keyErrMsg string }{ { ref: meta.GitCredentialsSecretReference, notFoundState: types.InvalidGitCredentialsSecretReference, - refType: "GitCredentialsSecretReference", + isSecret: true, + neededKeys: []string{GitCredsKnownHosts, v1.SSHAuthPrivateKey}, + errMsg: "Failed to get git credentials secret", + keyErrMsg: "not in git credentials secret", }, { ref: meta.TerraformCredentialsSecretReference, notFoundState: types.InvalidTerraformCredentialsSecretReference, - refType: "TerraformCredentialsSecretReference", + isSecret: true, + neededKeys: []string{TerraformCredentials}, + errMsg: "Failed to get terraform credentials secret", + keyErrMsg: "not in terraform credentials secret", }, { ref: meta.TerraformRCConfigMapReference, notFoundState: types.InvalidTerraformRCConfigMapReference, - refType: "TerraformRCConfigMapReference", + isSecret: false, + neededKeys: []string{TerraformRegistryConfig}, + errMsg: "Failed to get the terraform registry config configmap", + keyErrMsg: "not in terraform registry configuration configmap", }, { ref: meta.TerraformCredentialsHelperConfigMapReference, notFoundState: types.InvalidTerraformCredentialsHelperConfigMapReference, - refType: "TerraformCredentialsHelperConfigMapReference", + isSecret: false, + neededKeys: []string{}, + errMsg: "Failed to get the terraform credentials helper configmap", + keyErrMsg: "", }, } - - var checkErr error - var checkErrFlag bool for _, check := range secretConfigMapToCheck { if check.ref != nil { - switch check.refType { - case "GitCredentialsSecretReference": - gitCreds, err := GetGitCredentialsSecret(ctx, k8sClient, check.ref) - if gitCreds == nil { - checkErr = err - checkErrFlag = true - } - case "TerraformCredentialsSecretReference": - terraformCreds, err := GetTerraformCredentialsSecret(ctx, k8sClient, check.ref) - if terraformCreds == nil { - checkErr = err - checkErrFlag = true - } - case "TerraformRCConfigMapReference": - terraformRegistryConfig, err := GetTerraformRCConfigMap(ctx, k8sClient, check.ref) - if terraformRegistryConfig == nil { - checkErr = err - checkErrFlag = true - } - case "TerraformCredentialsHelperConfigMapReference": - terraformCredentialsHelper, err := GetTerraformCredentialsHelperConfigMap(ctx, k8sClient, check.ref) - if terraformCredentialsHelper == nil { - checkErr = err - checkErrFlag = true - } - } - - if checkErrFlag { + var object metav1.Object + var err error + object, err = GetSecretOrConfigMap(ctx, k8sClient, check.isSecret, check.ref, check.neededKeys, check.errMsg, check.keyErrMsg) + if object == nil { msg := string(check.notFoundState) - if checkErr != nil { - msg = checkErr.Error() + if err != nil { + msg = err.Error() } if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, check.notFoundState, msg); updateStatusErr != nil { return errors.Wrap(updateStatusErr, msg) @@ -1019,21 +1010,36 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume { inputTFConfigurationVolume := meta.createConfigurationVolume() tfBackendVolume := meta.createTFBackendVolume() executorVolumes := []v1.Volume{workingVolume, inputTFConfigurationVolume, tfBackendVolume} - if meta.GitCredentialsSecretReference != nil { - gitAuthConfigVolume := meta.createSecretOrConfigMapVolume(true, meta.GitCredentialsSecretReference.Name, GitAuthConfigVolumeName) - executorVolumes = append(executorVolumes, gitAuthConfigVolume) - } - if meta.TerraformCredentialsSecretReference != nil { - terraformCredentialsConfigVolume := meta.createSecretOrConfigMapVolume(true, meta.TerraformCredentialsSecretReference.Name, TerraformCredentialsConfigVolumeName) - executorVolumes = append(executorVolumes, terraformCredentialsConfigVolume) - } - if meta.TerraformRCConfigMapReference != nil { - terraformRegistryConfigVolume := meta.createSecretOrConfigMapVolume(false, meta.TerraformRCConfigMapReference.Name, TerraformRCConfigVolumeName) - executorVolumes = append(executorVolumes, terraformRegistryConfigVolume) + SecretOrConfigMapReferences := []struct { + ref *v1.SecretReference + volumeName string + isSecret bool + }{ + { + ref: meta.GitCredentialsSecretReference, + volumeName: GitAuthConfigVolumeName, + isSecret: true, + }, + { + ref: meta.TerraformCredentialsSecretReference, + volumeName: TerraformCredentialsConfigVolumeName, + isSecret: true, + }, + { + ref: meta.TerraformRCConfigMapReference, + volumeName: TerraformRCConfigVolumeName, + isSecret: false, + }, + { + ref: meta.TerraformCredentialsHelperConfigMapReference, + volumeName: TerraformCredentialsHelperConfigVolumeName, + isSecret: false, + }, } - if meta.TerraformCredentialsHelperConfigMapReference != nil { - terraformCredentialsHelperConfigVolume := meta.createSecretOrConfigMapVolume(false, meta.TerraformCredentialsHelperConfigMapReference.Name, TerraformCredentialsHelperConfigVolumeName) - executorVolumes = append(executorVolumes, terraformCredentialsHelperConfigVolume) + for _, ref := range SecretOrConfigMapReferences { + if ref.ref != nil { + executorVolumes = append(executorVolumes, meta.createSecretOrConfigMapVolume(ref.isSecret, ref.ref.Name, ref.volumeName)) + } } return executorVolumes } @@ -1477,54 +1483,7 @@ func (meta *TFConfigurationMeta) RenderConfiguration(configuration *v1beta2.Conf } } -// GetGitCredentialsSecret will get the secret containing the SSH private key & known_hosts -func GetGitCredentialsSecret(ctx context.Context, k8sClient client.Client, secretRef *v1.SecretReference) (*v1.Secret, error) { - needSecretKeys := []string{GitCredsKnownHosts, v1.SSHAuthPrivateKey} - errMsg := "Failed to get git credentials secret" - keyErrMsg := "not in git credentials secret" - secret, _, err := GetSecretOrConfigMap(ctx, k8sClient, true, secretRef, needSecretKeys, errMsg, keyErrMsg) - if secret != nil { - return secret, nil - } - return nil, err -} - -// GetTerraformCredentialsSecret will get the secret containing the terraform credentials -func GetTerraformCredentialsSecret(ctx context.Context, k8sClient client.Client, secretRef *v1.SecretReference) (*v1.Secret, error) { - needSecretKeys := []string{TerraformCredentials} - errMsg := "Failed to get terraform credentials secret" - keyErrMsg := "not in terraform credentials secret" - secret, _, err := GetSecretOrConfigMap(ctx, k8sClient, true, secretRef, needSecretKeys, errMsg, keyErrMsg) - if secret != nil { - return secret, nil - } - return nil, err -} - -// GetTerraformRCConfigMap will get the config map containing the terraform registry configuration -func GetTerraformRCConfigMap(ctx context.Context, k8sClient client.Client, configMapRef *v1.SecretReference) (*v1.ConfigMap, error) { - neededKeys := []string{TerraformRegistryConfig} - errMsg := "Failed to get the terraform registry config configmap" - keyErrMsg := "not in terraform registry configuration configmap" - _, configMap, err := GetSecretOrConfigMap(ctx, k8sClient, false, configMapRef, neededKeys, errMsg, keyErrMsg) - if configMap != nil { - return configMap, nil - } - return nil, err -} - -// GetTerraformCredentialsHelperConfigMap get the config map containing the terraform credentials helper -func GetTerraformCredentialsHelperConfigMap(ctx context.Context, k8sClient client.Client, configMapRef *v1.SecretReference) (*v1.ConfigMap, error) { - errMsg := "Failed to get the terraform credentials helper configmap" - neededKeys := []string{} - _, configMap, err := GetSecretOrConfigMap(ctx, k8sClient, false, configMapRef, neededKeys, errMsg, "") - if configMap != nil { - return configMap, nil - } - return nil, err -} - -func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret bool, ref *v1.SecretReference, neededKeys []string, errMsg string, keyErrMsg string) (*v1.Secret, *v1.ConfigMap, error) { +func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret bool, ref *v1.SecretReference, neededKeys []string, errMsg string, keyErrMsg string) (metav1.Object, error) { secret := &v1.Secret{} configMap := &v1.ConfigMap{} var err error @@ -1537,7 +1496,7 @@ func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret } if err != nil { klog.ErrorS(err, errMsg, "Name", ref.Name, "Namespace", ref.Namespace) - return nil, nil, errors.Wrap(err, errMsg) + return nil, errors.Wrap(err, errMsg) } if len(neededKeys) > 0 { for _, key := range neededKeys { @@ -1553,13 +1512,13 @@ func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret } if keyErr { keyErr := errors.Errorf("'%s' %s", key, keyErrMsg) - return nil, nil, keyErr + return nil, keyErr } } } if isSecret { - return secret, nil, nil + return secret, nil } - return nil, configMap, nil + return configMap, nil } diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index fbfdd47a..ccde41c9 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -2756,10 +2756,13 @@ func TestCheckGitCredentialsSecretReference(t *testing.T) { }, }, } + neededKeys := []string{GitCredsKnownHosts, corev1.SSHAuthPrivateKey} + errMsg := "Failed to get git credentials secret" + keyErrMsg := "not in git credentials secret" for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - sec, err := GetGitCredentialsSecret(ctx, tc.args.k8sClient, tc.args.GitCredentialsSecretReference) + sec, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, true, tc.args.GitCredentialsSecretReference, neededKeys, errMsg, keyErrMsg) if err != nil { assert.EqualError(t, err, tc.want.errMsg) } @@ -2862,9 +2865,13 @@ func TestCheckTerraformCredentialsSecretReference(t *testing.T) { }, } + neededKeys := []string{TerraformCredentials} + errMsg := "Failed to get terraform credentials secret" + keyErrMsg := "not in terraform credentials secret" + for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - sec, err := GetTerraformCredentialsSecret(ctx, tc.args.k8sClient, tc.args.TerraformCredentialsSecretReference) + sec, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, true, tc.args.TerraformCredentialsSecretReference, neededKeys, errMsg, keyErrMsg) if err != nil { assert.EqualError(t, err, tc.want.errMsg) @@ -2965,9 +2972,13 @@ func TestCheckTerraformRCConfigMapReference(t *testing.T) { }, } + neededKeys := []string{TerraformRegistryConfig} + errMsg := "Failed to get the terraform registry config configmap" + keyErrMsg := "not in terraform registry configuration configmap" + for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - configMap, err := GetTerraformRCConfigMap(ctx, tc.args.k8sClient, tc.args.TerraformRCConfigMapReference) + configMap, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, false, tc.args.TerraformRCConfigMapReference, neededKeys, errMsg, keyErrMsg) if err != nil { assert.EqualError(t, err, tc.want.errMsg) @@ -3041,9 +3052,13 @@ func TestTerraformCredentialsHelperConfigMap(t *testing.T) { }, } + neededKeys := []string{} + errMsg := "Failed to get the terraform credentials helper configmap" + keyErrMsg := "" + for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - configMap, err := GetTerraformCredentialsHelperConfigMap(ctx, tc.args.k8sClient, tc.args.TerraformCredentialsHelperConfigMapReference) + configMap, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, false, tc.args.TerraformCredentialsHelperConfigMapReference, neededKeys, errMsg, keyErrMsg) if err != nil { assert.EqualError(t, err, tc.want.errMsg) From 61f1aaae84e2ec7817e4237195559485d0c1b7f9 Mon Sep 17 00:00:00 2001 From: raradhakrishnan Date: Wed, 18 Jan 2023 13:04:14 -0500 Subject: [PATCH 13/15] feat: fixed review comments. Signed-off-by: raradhakrishnan --- controllers/configuration_controller.go | 55 +++++++++-------- controllers/configuration_controller_test.go | 62 +++++++++++++------- 2 files changed, 67 insertions(+), 50 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index ffd200e9..b3e03696 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -588,7 +588,7 @@ func (r *ConfigurationReconciler) preCheck(ctx context.Context, configuration *v } } - /* validate the follwing secretreferences and config maps. + /* validate the following secret references and configmap references. 1. GitCredentialsSecretReference 2. TerraformCredentialsSecretReference 3. TerraformRCConfigMapReference @@ -672,45 +672,42 @@ func (meta *TFConfigurationMeta) validateSecretAndConfigMap(ctx context.Context, neededKeys []string errMsg string keyErrMsg string + errKey string }{ { ref: meta.GitCredentialsSecretReference, notFoundState: types.InvalidGitCredentialsSecretReference, isSecret: true, neededKeys: []string{GitCredsKnownHosts, v1.SSHAuthPrivateKey}, - errMsg: "Failed to get git credentials secret", - keyErrMsg: "not in git credentials secret", + errKey: "git credentials", }, { ref: meta.TerraformCredentialsSecretReference, notFoundState: types.InvalidTerraformCredentialsSecretReference, isSecret: true, neededKeys: []string{TerraformCredentials}, - errMsg: "Failed to get terraform credentials secret", - keyErrMsg: "not in terraform credentials secret", + errKey: "terraform credentials", }, { ref: meta.TerraformRCConfigMapReference, notFoundState: types.InvalidTerraformRCConfigMapReference, isSecret: false, neededKeys: []string{TerraformRegistryConfig}, - errMsg: "Failed to get the terraform registry config configmap", - keyErrMsg: "not in terraform registry configuration configmap", + errKey: "terraformrc configuration", }, { ref: meta.TerraformCredentialsHelperConfigMapReference, notFoundState: types.InvalidTerraformCredentialsHelperConfigMapReference, isSecret: false, neededKeys: []string{}, - errMsg: "Failed to get the terraform credentials helper configmap", - keyErrMsg: "", + errKey: "terraform credentials helper", }, } for _, check := range secretConfigMapToCheck { if check.ref != nil { var object metav1.Object var err error - object, err = GetSecretOrConfigMap(ctx, k8sClient, check.isSecret, check.ref, check.neededKeys, check.errMsg, check.keyErrMsg) + object, err = GetSecretOrConfigMap(ctx, k8sClient, check.isSecret, check.ref, check.neededKeys, check.errKey) if object == nil { msg := string(check.notFoundState) if err != nil { @@ -1010,7 +1007,7 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume { inputTFConfigurationVolume := meta.createConfigurationVolume() tfBackendVolume := meta.createTFBackendVolume() executorVolumes := []v1.Volume{workingVolume, inputTFConfigurationVolume, tfBackendVolume} - SecretOrConfigMapReferences := []struct { + secretOrConfigMapReferences := []struct { ref *v1.SecretReference volumeName string isSecret bool @@ -1036,7 +1033,7 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume { isSecret: false, }, } - for _, ref := range SecretOrConfigMapReferences { + for _, ref := range secretOrConfigMapReferences { if ref.ref != nil { executorVolumes = append(executorVolumes, meta.createSecretOrConfigMapVolume(ref.isSecret, ref.ref.Name, ref.volumeName)) } @@ -1483,42 +1480,44 @@ func (meta *TFConfigurationMeta) RenderConfiguration(configuration *v1beta2.Conf } } -func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret bool, ref *v1.SecretReference, neededKeys []string, errMsg string, keyErrMsg string) (metav1.Object, error) { +func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret bool, ref *v1.SecretReference, neededKeys []string, errKey string) (metav1.Object, error) { secret := &v1.Secret{} configMap := &v1.ConfigMap{} var err error + // key to determine if it is a secret or config map + var typeKey string if isSecret { namespacedName := client.ObjectKey{Name: ref.Name, Namespace: ref.Namespace} err = k8sClient.Get(ctx, namespacedName, secret) + typeKey = "secret" } else { namespacedName := client.ObjectKey{Name: ref.Name, Namespace: ref.Namespace} err = k8sClient.Get(ctx, namespacedName, configMap) + typeKey = "configmap" } + errMsg := fmt.Sprintf("Failed to get %s %s", errKey, typeKey) if err != nil { klog.ErrorS(err, errMsg, "Name", ref.Name, "Namespace", ref.Namespace) return nil, errors.Wrap(err, errMsg) } - if len(neededKeys) > 0 { - for _, key := range neededKeys { - var keyErr bool - if isSecret { - if _, ok := secret.Data[key]; !ok { - keyErr = true - } - } else { - if _, ok := configMap.Data[key]; !ok { - keyErr = true - } + for _, key := range neededKeys { + var keyErr bool + if isSecret { + if _, ok := secret.Data[key]; !ok { + keyErr = true } - if keyErr { - keyErr := errors.Errorf("'%s' %s", key, keyErrMsg) - return nil, keyErr + } else { + if _, ok := configMap.Data[key]; !ok { + keyErr = true } } + if keyErr { + keyErr := errors.Errorf("'%s' %s", key, fmt.Sprintf("not in %s %s", errKey, typeKey)) + return nil, keyErr + } } if isSecret { return secret, nil } - return configMap, nil } diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index ccde41c9..9d486665 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1639,7 +1639,7 @@ func TestAssembleTerraformJobWithGitCredentialsSecretRef(t *testing.T) { assert.Contains(t, spec.Volumes, gitAuthSecretVolume) } -func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsSecretRef(t *testing.T) { +func TestAssembleTerraformJobWithTerraformRCAndCredentials(t *testing.T) { meta := &TFConfigurationMeta{ Name: "a", ConfigurationCMName: "b", @@ -1656,6 +1656,10 @@ func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsSecretRef( Namespace: "default", Name: "terraform-credentials", }, + TerraformCredentialsHelperConfigMapReference: &corev1.SecretReference{ + Namespace: "default", + Name: "terraform-credentials-helper", + }, } job := meta.assembleTerraformJob(TerraformApply) @@ -1686,15 +1690,33 @@ func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsSecretRef( MountPath: TerraformCredentialsConfigVolumeMountPath, } + terraformCredentialsHelperConfigVolume := corev1.Volume{Name: TerraformCredentialsHelperConfigVolumeName} + terraformCredentialsHelperConfigVolume.ConfigMap = &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "terraform-credentials-helper", + }, + DefaultMode: &terraformSecretDefaultMode, + } + + terraformCredentialsHelperConfigVolumeMount := corev1.VolumeMount{ + Name: TerraformCredentialsHelperConfigVolumeName, + MountPath: TerraformCredentialsHelperConfigVolumeMountPath, + } + + assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformCredentialsHelperConfigVolumeMount) + assert.Contains(t, spec.Volumes, terraformCredentialsHelperConfigVolume) + assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformRegistryConfigVolumeMount) assert.Contains(t, spec.Volumes, terraformRegistryConfigMapVolume) assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformCredentialsSecretVolumeMount) assert.Contains(t, spec.Volumes, terraformCredentialsSecretVolume) + assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformRegistryConfigVolumeMount) + assert.Contains(t, spec.Volumes, terraformRegistryConfigMapVolume) } -func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsHelperConfigMapRef(t *testing.T) { +func TestAssembleTerraformJobWithTerraformRCAndCredentialsHelper(t *testing.T) { meta := &TFConfigurationMeta{ Name: "a", ConfigurationCMName: "b", @@ -2757,12 +2779,11 @@ func TestCheckGitCredentialsSecretReference(t *testing.T) { }, } neededKeys := []string{GitCredsKnownHosts, corev1.SSHAuthPrivateKey} - errMsg := "Failed to get git credentials secret" - keyErrMsg := "not in git credentials secret" + errKey := "git credentials" for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - sec, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, true, tc.args.GitCredentialsSecretReference, neededKeys, errMsg, keyErrMsg) + sec, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, true, tc.args.GitCredentialsSecretReference, neededKeys, errKey) if err != nil { assert.EqualError(t, err, tc.want.errMsg) } @@ -2830,11 +2851,11 @@ func TestCheckTerraformCredentialsSecretReference(t *testing.T) { k8sClient: k8sClient, TerraformCredentialsSecretReference: &corev1.SecretReference{ Namespace: "default", - Name: "terraform-credentials", + Name: "secret-not-exists", }, }, want: want{ - errMsg: "Failed to get terraform credentials secret: secrets \"terraform-credentials\" not found", + errMsg: "Failed to get terraform credentials secret: secrets \"secret-not-exists\" not found", }, }, { @@ -2866,12 +2887,11 @@ func TestCheckTerraformCredentialsSecretReference(t *testing.T) { } neededKeys := []string{TerraformCredentials} - errMsg := "Failed to get terraform credentials secret" - keyErrMsg := "not in terraform credentials secret" + errKey := "terraform credentials" for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - sec, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, true, tc.args.TerraformCredentialsSecretReference, neededKeys, errMsg, keyErrMsg) + sec, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, true, tc.args.TerraformCredentialsSecretReference, neededKeys, errKey) if err != nil { assert.EqualError(t, err, tc.want.errMsg) @@ -2937,11 +2957,11 @@ func TestCheckTerraformRCConfigMapReference(t *testing.T) { k8sClient: k8sClient, TerraformRCConfigMapReference: &corev1.SecretReference{ Namespace: "default", - Name: "terraform-registry", + Name: "configmap-not-exists", }, }, want: want{ - errMsg: "Failed to get the terraform registry config configmap: configmaps \"terraform-registry\" not found", + errMsg: "Failed to get terraformrc configuration configmap: configmaps \"configmap-not-exists\" not found", }, }, { @@ -2954,7 +2974,7 @@ func TestCheckTerraformRCConfigMapReference(t *testing.T) { }, }, want: want{ - errMsg: fmt.Sprintf("'%s' not in terraform registry configuration configmap", TerraformRegistryConfig), + errMsg: fmt.Sprintf("'%s' not in terraformrc configuration configmap", TerraformRegistryConfig), }, }, { @@ -2973,12 +2993,11 @@ func TestCheckTerraformRCConfigMapReference(t *testing.T) { } neededKeys := []string{TerraformRegistryConfig} - errMsg := "Failed to get the terraform registry config configmap" - keyErrMsg := "not in terraform registry configuration configmap" + errKey := "terraformrc configuration" for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - configMap, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, false, tc.args.TerraformRCConfigMapReference, neededKeys, errMsg, keyErrMsg) + configMap, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, false, tc.args.TerraformRCConfigMapReference, neededKeys, errKey) if err != nil { assert.EqualError(t, err, tc.want.errMsg) @@ -3034,7 +3053,7 @@ func TestTerraformCredentialsHelperConfigMap(t *testing.T) { }, }, want: want{ - errMsg: "Failed to get the terraform credentials helper configmap: configmaps \"terraform-registry\" not found", + errMsg: "Failed to get terraform credentials helper configmap: configmaps \"terraform-registry\" not found", }, }, { @@ -3053,12 +3072,11 @@ func TestTerraformCredentialsHelperConfigMap(t *testing.T) { } neededKeys := []string{} - errMsg := "Failed to get the terraform credentials helper configmap" - keyErrMsg := "" + errKey := "terraform credentials helper" for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - configMap, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, false, tc.args.TerraformCredentialsHelperConfigMapReference, neededKeys, errMsg, keyErrMsg) + configMap, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, false, tc.args.TerraformCredentialsHelperConfigMapReference, neededKeys, errKey) if err != nil { assert.EqualError(t, err, tc.want.errMsg) @@ -3214,7 +3232,7 @@ func TestCheckValidateSecretAndConfigMap(t *testing.T) { }, }, want: want{ - errMsg: "Failed to get the terraform credentials helper configmap: configmaps \"terraform-registry\" not found", + errMsg: "Failed to get terraform credentials helper configmap: configmaps \"terraform-registry\" not found", }, }, { @@ -3248,7 +3266,7 @@ func TestCheckValidateSecretAndConfigMap(t *testing.T) { }, }, want: want{ - errMsg: "Failed to get the terraform registry config configmap: configmaps \"terraform-registry\" not found", + errMsg: "Failed to get terraformrc configuration configmap: configmaps \"terraform-registry\" not found", }, }, } From 07d485407ba5f3c1590663279d8c9512ec61d443 Mon Sep 17 00:00:00 2001 From: Ramesh Krishna Date: Wed, 18 Jan 2023 22:12:51 -0500 Subject: [PATCH 14/15] Update controllers/configuration_controller.go Co-authored-by: qiaozp <47812250+chivalryq@users.noreply.github.com> Signed-off-by: raradhakrishnan --- controllers/configuration_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index b3e03696..911f3087 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -1512,7 +1512,7 @@ func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret } } if keyErr { - keyErr := errors.Errorf("'%s' %s", key, fmt.Sprintf("not in %s %s", errKey, typeKey)) + keyErr := errors.Errorf("'%s' not in %s %s", key, errKey, typeKey) return nil, keyErr } } From 5b3fffb051ef1b5df7d64a135c3d1e2a0083ed29 Mon Sep 17 00:00:00 2001 From: Ramesh Krishna Date: Wed, 18 Jan 2023 22:13:04 -0500 Subject: [PATCH 15/15] Update controllers/configuration_controller.go Co-authored-by: qiaozp <47812250+chivalryq@users.noreply.github.com> Signed-off-by: raradhakrishnan --- controllers/configuration_controller.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 911f3087..f2ed61c2 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -670,8 +670,6 @@ func (meta *TFConfigurationMeta) validateSecretAndConfigMap(ctx context.Context, notFoundState types.ConfigurationState isSecret bool neededKeys []string - errMsg string - keyErrMsg string errKey string }{ {