Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if mandatory secrets/configmaps exist #799

Merged
merged 2 commits into from
Feb 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions pkg/canary/config_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,31 +146,29 @@ func (ct *ConfigTracker) GetTargetConfigs(cd *flaggerv1.Canary) (map[string]Conf
return nil, fmt.Errorf("TargetRef.Kind invalid: %s", cd.Spec.TargetRef.Kind)
}

type void struct{}
var member void
secretNames := map[string]void{}
configMapNames := map[string]void{}
secretNames := make(map[string]bool)
configMapNames := make(map[string]bool)

// scan volumes
for _, volume := range vs {
if cmv := volume.ConfigMap; cmv != nil {
name := cmv.Name
configMapNames[name] = member
configMapNames[name] = fieldIsMandatory(cmv.Optional)
}
if sv := volume.Secret; sv != nil {
name := sv.SecretName
secretNames[name] = member
secretNames[name] = fieldIsMandatory(sv.Optional)
}

if projected := volume.Projected; projected != nil {
for _, source := range projected.Sources {
if cmv := source.ConfigMap; cmv != nil {
name := cmv.Name
configMapNames[name] = member
configMapNames[name] = fieldIsMandatory(cmv.Optional)
}
if sv := source.Secret; sv != nil {
name := sv.Name
secretNames[name] = member
secretNames[name] = fieldIsMandatory(sv.Optional)
}
}
}
Expand All @@ -183,10 +181,10 @@ func (ct *ConfigTracker) GetTargetConfigs(cd *flaggerv1.Canary) (map[string]Conf
switch {
case env.ValueFrom.ConfigMapKeyRef != nil:
name := env.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name
configMapNames[name] = member
configMapNames[name] = fieldIsMandatory(env.ValueFrom.ConfigMapKeyRef.Optional)
case env.ValueFrom.SecretKeyRef != nil:
name := env.ValueFrom.SecretKeyRef.LocalObjectReference.Name
secretNames[name] = member
secretNames[name] = fieldIsMandatory(env.ValueFrom.SecretKeyRef.Optional)
}
}
}
Expand All @@ -195,30 +193,37 @@ func (ct *ConfigTracker) GetTargetConfigs(cd *flaggerv1.Canary) (map[string]Conf
switch {
case envFrom.ConfigMapRef != nil:
name := envFrom.ConfigMapRef.LocalObjectReference.Name
configMapNames[name] = member
configMapNames[name] = fieldIsMandatory(envFrom.ConfigMapRef.Optional)
case envFrom.SecretRef != nil:
name := envFrom.SecretRef.LocalObjectReference.Name
secretNames[name] = member
secretNames[name] = fieldIsMandatory(envFrom.SecretRef.Optional)
}
}
}

res := make(map[string]ConfigRef)

for configMapName := range configMapNames {
for configMapName, required := range configMapNames {
config, err := ct.getRefFromConfigMap(configMapName, cd.Namespace)
if err != nil {
ct.Logger.Errorf("getRefFromConfigMap failed: %v", err)
if required {
return nil, fmt.Errorf("configmap %s.%s get query error: %w", configMapName, cd.Namespace, err)
}
ct.Logger.Errorf("configmap %s.%s get query failed: %w", configMapName, cd.Namespace, err)
continue
}
if config != nil {
res[config.GetName()] = *config
}
}
for secretName := range secretNames {

for secretName, required := range secretNames {
secret, err := ct.getRefFromSecret(secretName, cd.Namespace)
if err != nil {
ct.Logger.Errorf("getRefFromSecret failed: %v", err)
if required {
return nil, fmt.Errorf("secret %s.%s get query error: %v", secretName, cd.Namespace, err)
}
ct.Logger.Errorf("secret %s.%s get query failed: %v", secretName, cd.Namespace, err)
continue
}
if secret != nil {
Expand All @@ -229,6 +234,13 @@ func (ct *ConfigTracker) GetTargetConfigs(cd *flaggerv1.Canary) (map[string]Conf
return res, nil
}

func fieldIsMandatory(p *bool) bool {
if p == nil {
return false
}
return !*p
}

// GetConfigRefs returns a map of configs and their checksum
func (ct *ConfigTracker) GetConfigRefs(cd *flaggerv1.Canary) (*map[string]string, error) {
res := make(map[string]string)
Expand Down
29 changes: 29 additions & 0 deletions pkg/canary/config_tracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@ package canary

import (
"context"
"errors"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
k8sTesting "k8s.io/client-go/testing"
)

func TestConfigIsDisabled(t *testing.T) {
Expand Down Expand Up @@ -303,3 +306,29 @@ func TestConfigTracker_Secrets(t *testing.T) {
assert.True(t, originalVolPresent, "Volume for original secret with disabled tracking should be present")
})
}

func TestConfigTracker_HasConfigChanged_ShouldReturnErrorWhenAPIServerIsDown(t *testing.T) {
t.Run("secret", func(t *testing.T) {
dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks, kubeClient := newCustomizableFixture(dc)

kubeClient.PrependReactor("get", "secrets", func(action k8sTesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("server error")
})

_, err := mocks.controller.configTracker.HasConfigChanged(mocks.canary)
assert.Error(t, err)
})

t.Run("configmap", func(t *testing.T) {
dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks, kubeClient := newCustomizableFixture(dc)

kubeClient.PrependReactor("get", "configmaps", func(action k8sTesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("server error")
})

_, err := mocks.controller.configTracker.HasConfigChanged(mocks.canary)
assert.Error(t, err)
})
}
10 changes: 9 additions & 1 deletion pkg/canary/deployment_fixture_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ func (d deploymentControllerFixture) initializeCanary(t *testing.T) {
}

func newDeploymentFixture(dc deploymentConfigs) deploymentControllerFixture {
fixture, _ := newCustomizableFixture(dc)
return fixture
}

func newCustomizableFixture(dc deploymentConfigs) (deploymentControllerFixture, *fake.Clientset) {
// init canary
cc := canaryConfigs{targetName: dc.name}
canary := newDeploymentControllerTestCanary(cc)
Expand Down Expand Up @@ -121,7 +126,7 @@ func newDeploymentFixture(dc deploymentConfigs) deploymentControllerFixture {
logger: logger,
flaggerClient: flaggerClient,
kubeClient: kubeClient,
}
}, kubeClient
}

func newDeploymentControllerTestConfigMap() *corev1.ConfigMap {
Expand Down Expand Up @@ -350,6 +355,7 @@ func newDeploymentControllerTestCanary(cc canaryConfigs) *flaggerv1.Canary {
}

func newDeploymentControllerTest(dc deploymentConfigs) *appsv1.Deployment {
var optional bool = false
d := &appsv1.Deployment{
TypeMeta: metav1.TypeMeta{APIVersion: appsv1.SchemeGroupVersion.String()},
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -473,6 +479,7 @@ func newDeploymentControllerTest(dc deploymentConfigs) *appsv1.Deployment {
LocalObjectReference: corev1.LocalObjectReference{
Name: "podinfo-config-vol",
},
Optional: &optional,
},
},
},
Expand All @@ -481,6 +488,7 @@ func newDeploymentControllerTest(dc deploymentConfigs) *appsv1.Deployment {
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: "podinfo-secret-vol",
Optional: &optional,
},
},
},
Expand Down