diff --git a/pkg/canary/config_tracker.go b/pkg/canary/config_tracker.go index 23e80feb1..77737d45d 100644 --- a/pkg/canary/config_tracker.go +++ b/pkg/canary/config_tracker.go @@ -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) } } } @@ -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) } } } @@ -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 { @@ -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) diff --git a/pkg/canary/config_tracker_test.go b/pkg/canary/config_tracker_test.go index 3399db95b..f229a5afb 100644 --- a/pkg/canary/config_tracker_test.go +++ b/pkg/canary/config_tracker_test.go @@ -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) { @@ -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) + }) +} diff --git a/pkg/canary/deployment_fixture_test.go b/pkg/canary/deployment_fixture_test.go index 14e5eff76..eb071eead 100644 --- a/pkg/canary/deployment_fixture_test.go +++ b/pkg/canary/deployment_fixture_test.go @@ -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) @@ -121,7 +126,7 @@ func newDeploymentFixture(dc deploymentConfigs) deploymentControllerFixture { logger: logger, flaggerClient: flaggerClient, kubeClient: kubeClient, - } + }, kubeClient } func newDeploymentControllerTestConfigMap() *corev1.ConfigMap { @@ -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{ @@ -473,6 +479,7 @@ func newDeploymentControllerTest(dc deploymentConfigs) *appsv1.Deployment { LocalObjectReference: corev1.LocalObjectReference{ Name: "podinfo-config-vol", }, + Optional: &optional, }, }, }, @@ -481,6 +488,7 @@ func newDeploymentControllerTest(dc deploymentConfigs) *appsv1.Deployment { VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ SecretName: "podinfo-secret-vol", + Optional: &optional, }, }, },