Skip to content

Commit 93ca018

Browse files
ramekris3163ramekris3163
committed
feat: fixed review comments.
Signed-off-by: raradhakrishnan <[email protected]>
1 parent 439672d commit 93ca018

File tree

2 files changed

+67
-50
lines changed

2 files changed

+67
-50
lines changed

‎controllers/configuration_controller.go

+27-28
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ func (r *ConfigurationReconciler) preCheck(ctx context.Context, configuration *v
588588
}
589589
}
590590

591-
/* validate the follwing secretreferences and config maps.
591+
/* validate the following secret references and configmap references.
592592
1. GitCredentialsSecretReference
593593
2. TerraformCredentialsSecretReference
594594
3. TerraformRCConfigMapReference
@@ -672,45 +672,42 @@ func (meta *TFConfigurationMeta) validateSecretAndConfigMap(ctx context.Context,
672672
neededKeys []string
673673
errMsg string
674674
keyErrMsg string
675+
errKey string
675676
}{
676677
{
677678
ref: meta.GitCredentialsSecretReference,
678679
notFoundState: types.InvalidGitCredentialsSecretReference,
679680
isSecret: true,
680681
neededKeys: []string{GitCredsKnownHosts, v1.SSHAuthPrivateKey},
681-
errMsg: "Failed to get git credentials secret",
682-
keyErrMsg: "not in git credentials secret",
682+
errKey: "git credentials",
683683
},
684684
{
685685
ref: meta.TerraformCredentialsSecretReference,
686686
notFoundState: types.InvalidTerraformCredentialsSecretReference,
687687
isSecret: true,
688688
neededKeys: []string{TerraformCredentials},
689-
errMsg: "Failed to get terraform credentials secret",
690-
keyErrMsg: "not in terraform credentials secret",
689+
errKey: "terraform credentials",
691690
},
692691
{
693692
ref: meta.TerraformRCConfigMapReference,
694693
notFoundState: types.InvalidTerraformRCConfigMapReference,
695694
isSecret: false,
696695
neededKeys: []string{TerraformRegistryConfig},
697-
errMsg: "Failed to get the terraform registry config configmap",
698-
keyErrMsg: "not in terraform registry configuration configmap",
696+
errKey: "terraformrc configuration",
699697
},
700698
{
701699
ref: meta.TerraformCredentialsHelperConfigMapReference,
702700
notFoundState: types.InvalidTerraformCredentialsHelperConfigMapReference,
703701
isSecret: false,
704702
neededKeys: []string{},
705-
errMsg: "Failed to get the terraform credentials helper configmap",
706-
keyErrMsg: "",
703+
errKey: "terraform credentials helper",
707704
},
708705
}
709706
for _, check := range secretConfigMapToCheck {
710707
if check.ref != nil {
711708
var object metav1.Object
712709
var err error
713-
object, err = GetSecretOrConfigMap(ctx, k8sClient, check.isSecret, check.ref, check.neededKeys, check.errMsg, check.keyErrMsg)
710+
object, err = GetSecretOrConfigMap(ctx, k8sClient, check.isSecret, check.ref, check.neededKeys, check.errKey)
714711
if object == nil {
715712
msg := string(check.notFoundState)
716713
if err != nil {
@@ -1010,7 +1007,7 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume {
10101007
inputTFConfigurationVolume := meta.createConfigurationVolume()
10111008
tfBackendVolume := meta.createTFBackendVolume()
10121009
executorVolumes := []v1.Volume{workingVolume, inputTFConfigurationVolume, tfBackendVolume}
1013-
SecretOrConfigMapReferences := []struct {
1010+
secretOrConfigMapReferences := []struct {
10141011
ref *v1.SecretReference
10151012
volumeName string
10161013
isSecret bool
@@ -1036,7 +1033,7 @@ func (meta *TFConfigurationMeta) assembleExecutorVolumes() []v1.Volume {
10361033
isSecret: false,
10371034
},
10381035
}
1039-
for _, ref := range SecretOrConfigMapReferences {
1036+
for _, ref := range secretOrConfigMapReferences {
10401037
if ref.ref != nil {
10411038
executorVolumes = append(executorVolumes, meta.createSecretOrConfigMapVolume(ref.isSecret, ref.ref.Name, ref.volumeName))
10421039
}
@@ -1483,42 +1480,44 @@ func (meta *TFConfigurationMeta) RenderConfiguration(configuration *v1beta2.Conf
14831480
}
14841481
}
14851482

1486-
func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret bool, ref *v1.SecretReference, neededKeys []string, errMsg string, keyErrMsg string) (metav1.Object, error) {
1483+
func GetSecretOrConfigMap(ctx context.Context, k8sClient client.Client, isSecret bool, ref *v1.SecretReference, neededKeys []string, errKey string) (metav1.Object, error) {
14871484
secret := &v1.Secret{}
14881485
configMap := &v1.ConfigMap{}
14891486
var err error
1487+
// key to determine if it is a secret or config map
1488+
var typeKey string
14901489
if isSecret {
14911490
namespacedName := client.ObjectKey{Name: ref.Name, Namespace: ref.Namespace}
14921491
err = k8sClient.Get(ctx, namespacedName, secret)
1492+
typeKey = "secret"
14931493
} else {
14941494
namespacedName := client.ObjectKey{Name: ref.Name, Namespace: ref.Namespace}
14951495
err = k8sClient.Get(ctx, namespacedName, configMap)
1496+
typeKey = "configmap"
14961497
}
1498+
errMsg := fmt.Sprintf("Failed to get %s %s", errKey, typeKey)
14971499
if err != nil {
14981500
klog.ErrorS(err, errMsg, "Name", ref.Name, "Namespace", ref.Namespace)
14991501
return nil, errors.Wrap(err, errMsg)
15001502
}
1501-
if len(neededKeys) > 0 {
1502-
for _, key := range neededKeys {
1503-
var keyErr bool
1504-
if isSecret {
1505-
if _, ok := secret.Data[key]; !ok {
1506-
keyErr = true
1507-
}
1508-
} else {
1509-
if _, ok := configMap.Data[key]; !ok {
1510-
keyErr = true
1511-
}
1503+
for _, key := range neededKeys {
1504+
var keyErr bool
1505+
if isSecret {
1506+
if _, ok := secret.Data[key]; !ok {
1507+
keyErr = true
15121508
}
1513-
if keyErr {
1514-
keyErr := errors.Errorf("'%s' %s", key, keyErrMsg)
1515-
return nil, keyErr
1509+
} else {
1510+
if _, ok := configMap.Data[key]; !ok {
1511+
keyErr = true
15161512
}
15171513
}
1514+
if keyErr {
1515+
keyErr := errors.Errorf("'%s' %s", key, fmt.Sprintf("not in %s %s", errKey, typeKey))
1516+
return nil, keyErr
1517+
}
15181518
}
15191519
if isSecret {
15201520
return secret, nil
15211521
}
1522-
15231522
return configMap, nil
15241523
}

‎controllers/configuration_controller_test.go

+40-22
Original file line numberDiff line numberDiff line change
@@ -1639,7 +1639,7 @@ func TestAssembleTerraformJobWithGitCredentialsSecretRef(t *testing.T) {
16391639
assert.Contains(t, spec.Volumes, gitAuthSecretVolume)
16401640
}
16411641

1642-
func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsSecretRef(t *testing.T) {
1642+
func TestAssembleTerraformJobWithTerraformRCAndCredentials(t *testing.T) {
16431643
meta := &TFConfigurationMeta{
16441644
Name: "a",
16451645
ConfigurationCMName: "b",
@@ -1656,6 +1656,10 @@ func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsSecretRef(
16561656
Namespace: "default",
16571657
Name: "terraform-credentials",
16581658
},
1659+
TerraformCredentialsHelperConfigMapReference: &corev1.SecretReference{
1660+
Namespace: "default",
1661+
Name: "terraform-credentials-helper",
1662+
},
16591663
}
16601664

16611665
job := meta.assembleTerraformJob(TerraformApply)
@@ -1686,15 +1690,33 @@ func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsSecretRef(
16861690
MountPath: TerraformCredentialsConfigVolumeMountPath,
16871691
}
16881692

1693+
terraformCredentialsHelperConfigVolume := corev1.Volume{Name: TerraformCredentialsHelperConfigVolumeName}
1694+
terraformCredentialsHelperConfigVolume.ConfigMap = &corev1.ConfigMapVolumeSource{
1695+
LocalObjectReference: corev1.LocalObjectReference{
1696+
Name: "terraform-credentials-helper",
1697+
},
1698+
DefaultMode: &terraformSecretDefaultMode,
1699+
}
1700+
1701+
terraformCredentialsHelperConfigVolumeMount := corev1.VolumeMount{
1702+
Name: TerraformCredentialsHelperConfigVolumeName,
1703+
MountPath: TerraformCredentialsHelperConfigVolumeMountPath,
1704+
}
1705+
1706+
assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformCredentialsHelperConfigVolumeMount)
1707+
assert.Contains(t, spec.Volumes, terraformCredentialsHelperConfigVolume)
1708+
16891709
assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformRegistryConfigVolumeMount)
16901710
assert.Contains(t, spec.Volumes, terraformRegistryConfigMapVolume)
16911711

16921712
assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformCredentialsSecretVolumeMount)
16931713
assert.Contains(t, spec.Volumes, terraformCredentialsSecretVolume)
16941714

1715+
assert.Contains(t, spec.InitContainers[0].VolumeMounts, terraformRegistryConfigVolumeMount)
1716+
assert.Contains(t, spec.Volumes, terraformRegistryConfigMapVolume)
16951717
}
16961718

1697-
func TestAssembleTerraformJobWithTerraformRegistryConfigAndCredentialsHelperConfigMapRef(t *testing.T) {
1719+
func TestAssembleTerraformJobWithTerraformRCAndCredentialsHelper(t *testing.T) {
16981720
meta := &TFConfigurationMeta{
16991721
Name: "a",
17001722
ConfigurationCMName: "b",
@@ -2757,12 +2779,11 @@ func TestCheckGitCredentialsSecretReference(t *testing.T) {
27572779
},
27582780
}
27592781
neededKeys := []string{GitCredsKnownHosts, corev1.SSHAuthPrivateKey}
2760-
errMsg := "Failed to get git credentials secret"
2761-
keyErrMsg := "not in git credentials secret"
2782+
errKey := "git credentials"
27622783

27632784
for _, tc := range testcases {
27642785
t.Run(tc.name, func(t *testing.T) {
2765-
sec, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, true, tc.args.GitCredentialsSecretReference, neededKeys, errMsg, keyErrMsg)
2786+
sec, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, true, tc.args.GitCredentialsSecretReference, neededKeys, errKey)
27662787
if err != nil {
27672788
assert.EqualError(t, err, tc.want.errMsg)
27682789
}
@@ -2830,11 +2851,11 @@ func TestCheckTerraformCredentialsSecretReference(t *testing.T) {
28302851
k8sClient: k8sClient,
28312852
TerraformCredentialsSecretReference: &corev1.SecretReference{
28322853
Namespace: "default",
2833-
Name: "terraform-credentials",
2854+
Name: "secret-not-exists",
28342855
},
28352856
},
28362857
want: want{
2837-
errMsg: "Failed to get terraform credentials secret: secrets \"terraform-credentials\" not found",
2858+
errMsg: "Failed to get terraform credentials secret: secrets \"secret-not-exists\" not found",
28382859
},
28392860
},
28402861
{
@@ -2866,12 +2887,11 @@ func TestCheckTerraformCredentialsSecretReference(t *testing.T) {
28662887
}
28672888

28682889
neededKeys := []string{TerraformCredentials}
2869-
errMsg := "Failed to get terraform credentials secret"
2870-
keyErrMsg := "not in terraform credentials secret"
2890+
errKey := "terraform credentials"
28712891

28722892
for _, tc := range testcases {
28732893
t.Run(tc.name, func(t *testing.T) {
2874-
sec, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, true, tc.args.TerraformCredentialsSecretReference, neededKeys, errMsg, keyErrMsg)
2894+
sec, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, true, tc.args.TerraformCredentialsSecretReference, neededKeys, errKey)
28752895

28762896
if err != nil {
28772897
assert.EqualError(t, err, tc.want.errMsg)
@@ -2937,11 +2957,11 @@ func TestCheckTerraformRCConfigMapReference(t *testing.T) {
29372957
k8sClient: k8sClient,
29382958
TerraformRCConfigMapReference: &corev1.SecretReference{
29392959
Namespace: "default",
2940-
Name: "terraform-registry",
2960+
Name: "configmap-not-exists",
29412961
},
29422962
},
29432963
want: want{
2944-
errMsg: "Failed to get the terraform registry config configmap: configmaps \"terraform-registry\" not found",
2964+
errMsg: "Failed to get terraformrc configuration configmap: configmaps \"configmap-not-exists\" not found",
29452965
},
29462966
},
29472967
{
@@ -2954,7 +2974,7 @@ func TestCheckTerraformRCConfigMapReference(t *testing.T) {
29542974
},
29552975
},
29562976
want: want{
2957-
errMsg: fmt.Sprintf("'%s' not in terraform registry configuration configmap", TerraformRegistryConfig),
2977+
errMsg: fmt.Sprintf("'%s' not in terraformrc configuration configmap", TerraformRegistryConfig),
29582978
},
29592979
},
29602980
{
@@ -2973,12 +2993,11 @@ func TestCheckTerraformRCConfigMapReference(t *testing.T) {
29732993
}
29742994

29752995
neededKeys := []string{TerraformRegistryConfig}
2976-
errMsg := "Failed to get the terraform registry config configmap"
2977-
keyErrMsg := "not in terraform registry configuration configmap"
2996+
errKey := "terraformrc configuration"
29782997

29792998
for _, tc := range testcases {
29802999
t.Run(tc.name, func(t *testing.T) {
2981-
configMap, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, false, tc.args.TerraformRCConfigMapReference, neededKeys, errMsg, keyErrMsg)
3000+
configMap, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, false, tc.args.TerraformRCConfigMapReference, neededKeys, errKey)
29823001

29833002
if err != nil {
29843003
assert.EqualError(t, err, tc.want.errMsg)
@@ -3034,7 +3053,7 @@ func TestTerraformCredentialsHelperConfigMap(t *testing.T) {
30343053
},
30353054
},
30363055
want: want{
3037-
errMsg: "Failed to get the terraform credentials helper configmap: configmaps \"terraform-registry\" not found",
3056+
errMsg: "Failed to get terraform credentials helper configmap: configmaps \"terraform-registry\" not found",
30383057
},
30393058
},
30403059
{
@@ -3053,12 +3072,11 @@ func TestTerraformCredentialsHelperConfigMap(t *testing.T) {
30533072
}
30543073

30553074
neededKeys := []string{}
3056-
errMsg := "Failed to get the terraform credentials helper configmap"
3057-
keyErrMsg := ""
3075+
errKey := "terraform credentials helper"
30583076

30593077
for _, tc := range testcases {
30603078
t.Run(tc.name, func(t *testing.T) {
3061-
configMap, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, false, tc.args.TerraformCredentialsHelperConfigMapReference, neededKeys, errMsg, keyErrMsg)
3079+
configMap, err := GetSecretOrConfigMap(ctx, tc.args.k8sClient, false, tc.args.TerraformCredentialsHelperConfigMapReference, neededKeys, errKey)
30623080

30633081
if err != nil {
30643082
assert.EqualError(t, err, tc.want.errMsg)
@@ -3214,7 +3232,7 @@ func TestCheckValidateSecretAndConfigMap(t *testing.T) {
32143232
},
32153233
},
32163234
want: want{
3217-
errMsg: "Failed to get the terraform credentials helper configmap: configmaps \"terraform-registry\" not found",
3235+
errMsg: "Failed to get terraform credentials helper configmap: configmaps \"terraform-registry\" not found",
32183236
},
32193237
},
32203238
{
@@ -3248,7 +3266,7 @@ func TestCheckValidateSecretAndConfigMap(t *testing.T) {
32483266
},
32493267
},
32503268
want: want{
3251-
errMsg: "Failed to get the terraform registry config configmap: configmaps \"terraform-registry\" not found",
3269+
errMsg: "Failed to get terraformrc configuration configmap: configmaps \"terraform-registry\" not found",
32523270
},
32533271
},
32543272
}

0 commit comments

Comments
 (0)