fix: old annotations only checked, but not deleted

Signed-off-by: Bence Csati <[email protected]>
bank-vaults · May 25, 2024 · 414fda5 · 414fda5
1 parent dc2dec7
commit 414fda5
Show file tree

Hide file tree

Showing 9 changed files with 140 additions and 583 deletions.
diff --git a/pkg/common/common.go b/pkg/common/common.go
@@ -29,7 +29,6 @@ const (
 	MutateAnnotation                      = "secrets-webhook.security.bank-vaults.io/mutate"
 	MutateProbesAnnotation                = "secrets-webhook.security.bank-vaults.io/mutate-probes"
 	ProviderAnnotation                    = "secrets-webhook.security.bank-vaults.io/provider"
-	CleanupOldAnnotationsAnnotation       = "secrets-webhook.security.bank-vaults.io/cleanup-old-annotations"
 
 	// Secret-init annotations
 	SecretInitDaemonAnnotation          = "secrets-webhook.security.bank-vaults.io/secret-init-daemon"

diff --git a/pkg/common/config.go b/pkg/common/config.go
@@ -18,7 +18,6 @@ import (
 	"strconv"
 
 	"github.com/spf13/viper"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // Config represents the configuration for the webhook
@@ -34,64 +33,72 @@ type Config struct {
 	Provider                    string
 }
 
-func LoadWebhookConfig(obj metav1.Object) Config {
+func LoadWebhookConfig(annotations map[string]string) Config {
 	config := Config{}
 
-	annotations := obj.GetAnnotations()
-
-	if val, ok := annotations[CleanupOldAnnotationsAnnotation]; ok {
-		ok, _ := strconv.ParseBool(val)
-		if ok {
-			annotations = handleDeprecatedWebhookAnnotations(annotations)
-		} else {
-			annotations = handleDeprecatedWebhookAnnotationsWithoutDelete(annotations)
-		}
-	}
-
+	// Do an early exit if the resource shouldn't be mutated
 	if val := annotations[MutateAnnotation]; val == "skip" {
 		config.Mutate = true
 
+		return config
+	} else if val := annotations[MutateAnnotationDeprecated]; val == "skip" {
+		config.Mutate = true
+
 		return config
 	}
 
 	if val, ok := annotations[PSPAllowPrivilegeEscalationAnnotation]; ok {
 		config.PspAllowPrivilegeEscalation, _ = strconv.ParseBool(val)
+	} else if val, ok := annotations[PSPAllowPrivilegeEscalationAnnotationDeprecated]; ok {
+		config.PspAllowPrivilegeEscalation, _ = strconv.ParseBool(val)
 	} else {
 		config.PspAllowPrivilegeEscalation, _ = strconv.ParseBool(viper.GetString(PSPAllowPrivilegeEscalationEnvVar))
 	}
 
 	if val, ok := annotations[RunAsNonRootAnnotation]; ok {
 		config.RunAsNonRoot, _ = strconv.ParseBool(val)
+	} else if val, ok := annotations[RunAsNonRootAnnotationDeprecated]; ok {
+		config.RunAsNonRoot, _ = strconv.ParseBool(val)
 	} else {
 		config.RunAsNonRoot, _ = strconv.ParseBool(viper.GetString(RunAsNonRootEnvVar))
 	}
 
 	if val, ok := annotations[RunAsUserAnnotation]; ok {
 		config.RunAsUser, _ = strconv.ParseInt(val, 10, 64)
+	} else if val, ok := annotations[RunAsUserAnnotationDeprecated]; ok {
+		config.RunAsUser, _ = strconv.ParseInt(val, 10, 64)
 	} else {
 		config.RunAsUser, _ = strconv.ParseInt(viper.GetString(RunAsUserEnvVar), 0, 64)
 	}
 
 	if val, ok := annotations[RunAsGroupAnnotation]; ok {
 		config.RunAsGroup, _ = strconv.ParseInt(val, 10, 64)
+	} else if val, ok := annotations[RunAsGroupAnnotationDeprecated]; ok {
+		config.RunAsGroup, _ = strconv.ParseInt(val, 10, 64)
 	} else {
 		config.RunAsGroup, _ = strconv.ParseInt(viper.GetString(RunAsGroupEnvVar), 0, 64)
 	}
 
 	if val, ok := annotations[ReadOnlyRootFsAnnotation]; ok {
 		config.ReadOnlyRootFilesystem, _ = strconv.ParseBool(val)
+	} else if val, ok := annotations[ReadOnlyRootFsAnnotationDeprecated]; ok {
+		config.ReadOnlyRootFilesystem, _ = strconv.ParseBool(val)
 	} else {
 		config.ReadOnlyRootFilesystem, _ = strconv.ParseBool(viper.GetString(ReadonlyRootFSEnvVar))
 	}
 
 	if val, ok := annotations[RegistrySkipVerifyAnnotation]; ok {
 		config.RegistrySkipVerify, _ = strconv.ParseBool(val)
+	} else if val, ok := annotations[RegistrySkipVerifyAnnotationDeprecated]; ok {
+		config.RegistrySkipVerify, _ = strconv.ParseBool(val)
 	} else {
 		config.RegistrySkipVerify, _ = strconv.ParseBool(viper.GetString(RegistrySkipVerifyEnvVar))
 	}
 
 	if val, ok := annotations[MutateProbesAnnotation]; ok {
 		config.MutateProbes, _ = strconv.ParseBool(val)
+	} else if val, ok := annotations[MutateProbesAnnotationDeprecated]; ok {
+		config.MutateProbes, _ = strconv.ParseBool(val)
 	}
 
 	if val, ok := annotations[ProviderAnnotation]; ok {
@@ -119,97 +126,3 @@ func SetConfigDefaults() {
 
 	viper.AutomaticEnv()
 }
-
-// This is implemented to preserve backwards compatibility with the deprecated annotations
-func handleDeprecatedWebhookAnnotations(annotations map[string]string) map[string]string {
-
-	if val, ok := annotations[MutateAnnotationDeprecated]; ok {
-		annotations[MutateAnnotation] = val
-		delete(annotations, MutateAnnotationDeprecated)
-
-		// Do an early exit if the resource shouldn't be mutated
-		if val == "skip" {
-			return annotations
-		}
-	}
-
-	if val, ok := annotations[PSPAllowPrivilegeEscalationAnnotationDeprecated]; ok {
-		annotations[PSPAllowPrivilegeEscalationAnnotation] = val
-		delete(annotations, PSPAllowPrivilegeEscalationAnnotationDeprecated)
-	}
-
-	if val, ok := annotations[RunAsNonRootAnnotationDeprecated]; ok {
-		annotations[RunAsNonRootAnnotation] = val
-		delete(annotations, RunAsNonRootAnnotationDeprecated)
-	}
-
-	if val, ok := annotations[RunAsUserAnnotationDeprecated]; ok {
-		annotations[RunAsUserAnnotation] = val
-		delete(annotations, RunAsUserAnnotationDeprecated)
-	}
-
-	if val, ok := annotations[RunAsGroupAnnotationDeprecated]; ok {
-		annotations[RunAsGroupAnnotation] = val
-		delete(annotations, RunAsGroupAnnotationDeprecated)
-	}
-
-	if val, ok := annotations[ReadOnlyRootFsAnnotationDeprecated]; ok {
-		annotations[ReadOnlyRootFsAnnotation] = val
-		delete(annotations, ReadOnlyRootFsAnnotationDeprecated)
-	}
-
-	if val, ok := annotations[RegistrySkipVerifyAnnotationDeprecated]; ok {
-		annotations[RegistrySkipVerifyAnnotation] = val
-		delete(annotations, RegistrySkipVerifyAnnotationDeprecated)
-	}
-
-	if val, ok := annotations[MutateProbesAnnotationDeprecated]; ok {
-		annotations[MutateProbesAnnotation] = val
-		delete(annotations, MutateProbesAnnotationDeprecated)
-	}
-
-	return annotations
-}
-
-// This is implemented to preserve backwards compatibility with the deprecated annotations
-func handleDeprecatedWebhookAnnotationsWithoutDelete(annotations map[string]string) map[string]string {
-
-	if val, ok := annotations[MutateAnnotationDeprecated]; ok {
-		annotations[MutateAnnotation] = val
-
-		// Do an early exit if the resource shouldn't be mutated
-		if val == "skip" {
-			return annotations
-		}
-	}
-
-	if val, ok := annotations[PSPAllowPrivilegeEscalationAnnotationDeprecated]; ok {
-		annotations[PSPAllowPrivilegeEscalationAnnotation] = val
-	}
-
-	if val, ok := annotations[RunAsNonRootAnnotationDeprecated]; ok {
-		annotations[RunAsNonRootAnnotation] = val
-	}
-
-	if val, ok := annotations[RunAsUserAnnotationDeprecated]; ok {
-		annotations[RunAsUserAnnotation] = val
-	}
-
-	if val, ok := annotations[RunAsGroupAnnotationDeprecated]; ok {
-		annotations[RunAsGroupAnnotation] = val
-	}
-
-	if val, ok := annotations[ReadOnlyRootFsAnnotationDeprecated]; ok {
-		annotations[ReadOnlyRootFsAnnotation] = val
-	}
-
-	if val, ok := annotations[RegistrySkipVerifyAnnotationDeprecated]; ok {
-		annotations[RegistrySkipVerifyAnnotation] = val
-	}
-
-	if val, ok := annotations[MutateProbesAnnotationDeprecated]; ok {
-		annotations[MutateProbesAnnotation] = val
-	}
-
-	return annotations
-}
diff --git a/pkg/common/config_test.go b/pkg/common/config_test.go
@@ -20,7 +20,6 @@ import (
 
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestLoadWebhookConfig(t *testing.T) {
@@ -33,7 +32,6 @@ func TestLoadWebhookConfig(t *testing.T) {
 		{
 			name: "Handle deprecated webhook annotations all",
 			annotations: map[string]string{
-				CleanupOldAnnotationsAnnotation:                 "true",
 				MutateAnnotationDeprecated:                      "false",
 				PSPAllowPrivilegeEscalationAnnotationDeprecated: "true",
 				RunAsNonRootAnnotationDeprecated:                "true",
@@ -57,7 +55,6 @@ func TestLoadWebhookConfig(t *testing.T) {
 		{
 			name: "Should stop parsing annotations if mutate is set to skip",
 			annotations: map[string]string{
-				CleanupOldAnnotationsAnnotation:                 "true",
 				MutateAnnotationDeprecated:                      "skip",
 				PSPAllowPrivilegeEscalationAnnotationDeprecated: "true",
 				RunAsGroupAnnotation:                            "1000",
@@ -80,7 +77,7 @@ func TestLoadWebhookConfig(t *testing.T) {
 				os.Clearenv()
 			})
 
-			whConfig := LoadWebhookConfig(&metav1.ObjectMeta{Annotations: ttp.annotations})
+			whConfig := LoadWebhookConfig(ttp.annotations)
 			assert.Equal(t, ttp.webhookConfigWant, whConfig)
 		})
 	}

diff --git a/pkg/common/secret_init_config.go b/pkg/common/secret_init_config.go
@@ -21,7 +21,6 @@ import (
 	"github.com/spf13/viper"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // SecretInitConfig represents the configuration for the secret-init container
@@ -39,57 +38,56 @@ type SecretInitConfig struct {
 	MemoryLimit     resource.Quantity
 }
 
-func LoadSecretInitConfig(obj metav1.Object) SecretInitConfig {
+func LoadSecretInitConfig(annotations map[string]string) SecretInitConfig {
 	setSecretInitConfigDefaults()
 	handleDeprecatedSecretInitEnvVars()
 
 	secretInitConfig := SecretInitConfig{}
 
-	annotations := obj.GetAnnotations()
-
-	if val, ok := annotations[CleanupOldAnnotationsAnnotation]; ok {
-		ok, _ := strconv.ParseBool(val)
-		if ok {
-			annotations = handleDeprecatedSecretInitAnnotations(annotations)
-		} else {
-			annotations = handleDeprecatedSecretInitAnnotationsWithoutDelete(annotations)
-		}
-	}
-
 	if val, ok := annotations[SecretInitDaemonAnnotation]; ok {
 		secretInitConfig.Daemon, _ = strconv.ParseBool(val)
+	} else if val, ok := annotations[VaultEnvDaemonAnnotationDeprecated]; ok {
+		secretInitConfig.Daemon, _ = strconv.ParseBool(val)
 	} else {
 		secretInitConfig.Daemon, _ = strconv.ParseBool(viper.GetString(SecretInitDaemonEnvVar))
 	}
 
 	if val, ok := annotations[SecretInitDelayAnnotation]; ok {
 		secretInitConfig.Delay, _ = time.ParseDuration(val)
+	} else if val, ok := annotations[VaultEnvDelayAnnotationDeprecated]; ok {
+		secretInitConfig.Delay, _ = time.ParseDuration(val)
 	} else {
 		secretInitConfig.Delay, _ = time.ParseDuration(viper.GetString(SecretInitDelayEnvVar))
 	}
 
 	if val, ok := annotations[SecretInitJSONLogAnnotation]; ok {
 		secretInitConfig.JSONLog = val
+	} else if val, ok := annotations[VaultEnvEnableJSONLogAnnotationDeprecated]; ok {
+		secretInitConfig.JSONLog = val
 	} else {
 		secretInitConfig.JSONLog = viper.GetString(SecretInitJSONLogEnvVar)
 	}
 
 	if val, ok := annotations[SecretInitImageAnnotation]; ok {
 		secretInitConfig.Image = val
+	} else if val, ok := annotations[VaultEnvImageAnnotationDeprecated]; ok {
+		secretInitConfig.Image = val
 	} else {
 		secretInitConfig.Image = viper.GetString(SecretInitImageEnvVar)
 	}
 
-	secretInitConfig.LogServer = viper.GetString(SecretInitLogServerEnvVar)
-
-	secretInitConfig.LogLevel = viper.GetString(SecretInitLogLevelEnvVar)
-
 	if val, ok := annotations[SecretInitImagePullPolicyAnnotation]; ok {
 		secretInitConfig.ImagePullPolicy = GetPullPolicy(val)
+	} else if val, ok := annotations[VaultEnvImagePullPolicyAnnotationDeprecated]; ok {
+		secretInitConfig.ImagePullPolicy = GetPullPolicy(val)
 	} else {
 		secretInitConfig.ImagePullPolicy = GetPullPolicy(viper.GetString(SecretInitImagePullPolicyEnvVar))
 	}
 
+	secretInitConfig.LogServer = viper.GetString(SecretInitLogServerEnvVar)
+
+	secretInitConfig.LogLevel = viper.GetString(SecretInitLogLevelEnvVar)
+
 	if val, err := resource.ParseQuantity(viper.GetString(SecretInitCPURequestEnvVar)); err == nil {
 		secretInitConfig.CPURequest = val
 	} else {
@@ -132,61 +130,6 @@ func setSecretInitConfigDefaults() {
 	viper.AutomaticEnv()
 }
 
-// This is implemented to preserve backwards compatibility with the deprecated annotations
-func handleDeprecatedSecretInitAnnotations(annotations map[string]string) map[string]string {
-	if val, ok := annotations[VaultEnvDaemonAnnotationDeprecated]; ok {
-		annotations[SecretInitDaemonAnnotation] = val
-		delete(annotations, VaultEnvDaemonAnnotationDeprecated)
-	}
-
-	if val, ok := annotations[VaultEnvDelayAnnotationDeprecated]; ok {
-		annotations[SecretInitDelayAnnotation] = val
-		delete(annotations, VaultEnvDelayAnnotationDeprecated)
-	}
-
-	if val, ok := annotations[VaultEnvEnableJSONLogAnnotationDeprecated]; ok {
-		annotations[SecretInitJSONLogAnnotation] = val
-		delete(annotations, VaultEnvEnableJSONLogAnnotationDeprecated)
-	}
-
-	if val, ok := annotations[VaultEnvImageAnnotationDeprecated]; ok {
-		annotations[SecretInitImageAnnotation] = val
-		delete(annotations, VaultEnvImageAnnotationDeprecated)
-	}
-
-	if val, ok := annotations[VaultEnvImagePullPolicyAnnotationDeprecated]; ok {
-		annotations[SecretInitImagePullPolicyAnnotation] = val
-		delete(annotations, VaultEnvImagePullPolicyAnnotationDeprecated)
-	}
-
-	return annotations
-}
-
-// This is implemented to preserve backwards compatibility with the deprecated annotations
-func handleDeprecatedSecretInitAnnotationsWithoutDelete(annotations map[string]string) map[string]string {
-	if val, ok := annotations[VaultEnvDaemonAnnotationDeprecated]; ok {
-		annotations[SecretInitDaemonAnnotation] = val
-	}
-
-	if val, ok := annotations[VaultEnvDelayAnnotationDeprecated]; ok {
-		annotations[SecretInitDelayAnnotation] = val
-	}
-
-	if val, ok := annotations[VaultEnvEnableJSONLogAnnotationDeprecated]; ok {
-		annotations[SecretInitJSONLogAnnotation] = val
-	}
-
-	if val, ok := annotations[VaultEnvImageAnnotationDeprecated]; ok {
-		annotations[SecretInitImageAnnotation] = val
-	}
-
-	if val, ok := annotations[VaultEnvImagePullPolicyAnnotationDeprecated]; ok {
-		annotations[SecretInitImagePullPolicyAnnotation] = val
-	}
-
-	return annotations
-}
-
 func handleDeprecatedSecretInitEnvVars() {
 	if val := viper.GetString(VaultEnvDaemonEnvVarDeprecated); val != "" {
 		viper.Set(SecretInitDaemonEnvVar, val)

diff --git a/pkg/common/secret_init_config_test.go b/pkg/common/secret_init_config_test.go
@@ -22,7 +22,6 @@ import (
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/api/resource"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestLoadSecretInitConfig(t *testing.T) {
@@ -35,7 +34,6 @@ func TestLoadSecretInitConfig(t *testing.T) {
 		{
 			name: "Handle deprecated secret init annotations all",
 			annotations: map[string]string{
-				CleanupOldAnnotationsAnnotation:             "true",
 				VaultEnvDaemonAnnotationDeprecated:          "true",
 				VaultEnvDelayAnnotationDeprecated:           "10s",
 				VaultEnvEnableJSONLogAnnotationDeprecated:   "true",
@@ -96,7 +94,7 @@ func TestLoadSecretInitConfig(t *testing.T) {
 				os.Clearenv()
 			})
 
-			secretInitConfig := LoadSecretInitConfig(&metav1.ObjectMeta{Annotations: ttp.annotations})
+			secretInitConfig := LoadSecretInitConfig(ttp.annotations)
 			assert.Equal(t, ttp.secretInitConfigWant, secretInitConfig)
 		})
 	}