Skip to content

Commit

Permalink
Respect optional param for ConfigMaps and Secrets (#562)
Browse files Browse the repository at this point in the history
* Respect optional in ConfigMaps and Secrets

* add test for resolveEnv

* remove name from testMetadata
  • Loading branch information
tomislater authored and ahmelsayed committed Jan 28, 2020
1 parent 104c71b commit 16cae71
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 0 deletions.
6 changes: 6 additions & 0 deletions pkg/handler/scale_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ func (h *ScaleHandler) resolveEnv(container *corev1.Container, namespace string)
for k, v := range configMap {
resolved[k] = v
}
} else if source.ConfigMapRef.Optional != nil && *source.ConfigMapRef.Optional {
// ignore error when ConfigMap is marked as optional
continue
} else {
return nil, fmt.Errorf("error reading config ref %s on namespace %s/: %s", source.ConfigMapRef, namespace, err)
}
Expand All @@ -68,6 +71,9 @@ func (h *ScaleHandler) resolveEnv(container *corev1.Container, namespace string)
for k, v := range secretsMap {
resolved[k] = v
}
} else if source.SecretRef.Optional != nil && *source.SecretRef.Optional {
// ignore error when Secret is marked as optional
continue
} else {
return nil, fmt.Errorf("error reading secret ref %s on namespace %s: %s", source.SecretRef, namespace, err)
}
Expand Down
123 changes: 123 additions & 0 deletions pkg/handler/scale_handler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package handler

import (
"testing"

corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

var (
namespace string = "test-namespace"
trueValue bool = true
falseValue bool = false
)

type testMetadata struct {
isError bool
comment string
container *corev1.Container
}

var testMetadatas = []testMetadata{
{
isError: true,
comment: "configmap does not exist, and it is not marked as an optional, there should be an error",
container: &corev1.Container{
EnvFrom: []corev1.EnvFromSource{{
ConfigMapRef: &corev1.ConfigMapEnvSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "do-not-exist-not-optional",
},
},
}},
},
},
{
isError: true,
comment: "secret does not exist, and it is not marked as an optional, there should be an error",
container: &corev1.Container{
EnvFrom: []corev1.EnvFromSource{{
SecretRef: &corev1.SecretEnvSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "do-not-exist-not-optional",
},
},
}},
},
},
{
isError: false,
comment: "configmap does not exist, but it is marked as an optional, there should not be an error",
container: &corev1.Container{
EnvFrom: []corev1.EnvFromSource{{
ConfigMapRef: &corev1.ConfigMapEnvSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "do-not-exist-but-optional",
},
Optional: &trueValue,
},
}},
},
},
{
isError: false,
comment: "secret does not exist, but it is marked as an optional, there should not be an error",
container: &corev1.Container{
EnvFrom: []corev1.EnvFromSource{{
SecretRef: &corev1.SecretEnvSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "do-not-exist-but-optional",
},
Optional: &trueValue,
},
}},
},
},
{
isError: true,
comment: "configmap does not exist, and it is not marked as an optional, there should be an error",
container: &corev1.Container{
EnvFrom: []corev1.EnvFromSource{{
ConfigMapRef: &corev1.ConfigMapEnvSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "do-not-exist-and-not-optional-explicitly",
},
Optional: &falseValue,
},
}},
},
},
{
isError: true,
comment: "secret does not exist, and it is not marked as an optional, there should be an error",
container: &corev1.Container{
EnvFrom: []corev1.EnvFromSource{{
SecretRef: &corev1.SecretEnvSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "do-not-exist-and-not-optional-explicitly",
},
Optional: &falseValue,
},
}},
},
},
}

func TestResolveNonExistingConfigMapsOrSecretsEnv(t *testing.T) {

for _, testData := range testMetadatas {
testScaleHandler := NewScaleHandler(fake.NewFakeClient(), scheme.Scheme)

_, err := testScaleHandler.resolveEnv(testData.container, namespace)

if err != nil && !testData.isError {
t.Errorf("Expected success because %s got error, %s", testData.comment, err)
}

if testData.isError && err == nil {
t.Errorf("Expected error because %s but got success, %#v", testData.comment, testData)
}
}
}

0 comments on commit 16cae71

Please sign in to comment.