Repository: GoogleCloudPlatform/cloud-sql-proxy-operator
Pull request #263 (2023-03-10 16:53:09)

fix: Validate the AdminServer.EnableAPIs[] field properly.
Validation generated by kubebuilder:Enum does not work with []string fields because it applies to the entire value, not the elements of the slice.


Patch:
diff --git a/config/crd/bases/cloudsql.cloud.google.com_authproxyworkloads.yaml b/config/crd/bases/cloudsql.cloud.google.com_authproxyworkloads.yaml
index 9438dc8..2da6adf 100644
--- a/config/crd/bases/cloudsql.cloud.google.com_authproxyworkloads.yaml
+++ b/config/crd/bases/cloudsql.cloud.google.com_authproxyworkloads.yaml
@@ -51,9 +51,6 @@ spec:
                       properties:
                         enableAPIs:
                           description: 'EnableAPIs specifies the list of admin APIs to enable. At least one API must be enabled. Possible values: - "Debug" will enable pprof debugging by setting the `--debug` cli flag. - "QuitQuitQuit" will enable pprof debugging by setting the `--quitquitquit` cli flag.'
-                          enum:
-                            - Debug
-                            - QuitQuitQuit
                           items:
                             type: string
                           minItems: 1
diff --git a/installer/cloud-sql-proxy-operator.yaml b/installer/cloud-sql-proxy-operator.yaml
index aaa3ff4..1f86325 100644
--- a/installer/cloud-sql-proxy-operator.yaml
+++ b/installer/cloud-sql-proxy-operator.yaml
@@ -69,9 +69,6 @@ spec:
                       properties:
                         enableAPIs:
                           description: 'EnableAPIs specifies the list of admin APIs to enable. At least one API must be enabled. Possible values: - "Debug" will enable pprof debugging by setting the `--debug` cli flag. - "QuitQuitQuit" will enable pprof debugging by setting the `--quitquitquit` cli flag.'
-                          enum:
-                            - Debug
-                            - QuitQuitQuit
                           items:
                             type: string
                           minItems: 1
diff --git a/internal/api/v1/authproxyworkload_types.go b/internal/api/v1/authproxyworkload_types.go
index 9e8ac37..a66e389 100644
--- a/internal/api/v1/authproxyworkload_types.go
+++ b/internal/api/v1/authproxyworkload_types.go
@@ -224,7 +224,6 @@ type AdminServerSpec struct {
 	// - "QuitQuitQuit" will enable pprof debugging by setting the `--quitquitquit`
 	//   cli flag.
 	//+kubebuilder:validation:MinItems:=1
-	//+kubebuilder:validation:Enum=Debug;QuitQuitQuit
 	EnableAPIs []string `json:"enableAPIs,omitempty"`
 }
 
diff --git a/internal/api/v1/authproxyworkload_webhook.go b/internal/api/v1/authproxyworkload_webhook.go
index b53d976..4e5bd32 100644
--- a/internal/api/v1/authproxyworkload_webhook.go
+++ b/internal/api/v1/authproxyworkload_webhook.go
@@ -112,10 +112,19 @@ func validateContainer(spec *AuthProxyContainerSpec, f *field.Path) field.ErrorL
 	}
 
 	var allErrs field.ErrorList
-	if spec.AdminServer != nil && len(spec.AdminServer.EnableAPIs) == 0 {
-		allErrs = append(allErrs, field.Invalid(
-			f.Child("adminServer", "enableAPIs"), nil,
-			"enableAPIs must have at least one valid element: Debug or QuitQuitQuit"))
+	if spec.AdminServer != nil {
+		if len(spec.AdminServer.EnableAPIs) == 0 {
+			allErrs = append(allErrs, field.Invalid(
+				f.Child("adminServer", "enableAPIs"), nil,
+				"enableAPIs must have at least one valid element: Debug or QuitQuitQuit"))
+		}
+		for i, v := range spec.AdminServer.EnableAPIs {
+			if v != "Debug" && v != "QuitQuitQuit" {
+				allErrs = append(allErrs, field.Invalid(
+					f.Child("adminServer", "enableAPIs", fmt.Sprintf("%d", i)), v,
+					"enableAPIs may contain the values \"Debug\" or \"QuitQuitQuit\""))
+			}
+		}
 	}
 	if spec.AdminServer != nil {
 		errors := apivalidation.IsValidPortNum(int(spec.AdminServer.Port))
@@ -251,7 +260,7 @@ func validateInstances(spec *[]InstanceSpec, f *field.Path) field.ErrorList {
 		return errs
 	}
 	for i, inst := range *spec {
-		ff := f.Child(fmt.Sprintf("[%d]", i))
+		ff := f.Child(fmt.Sprintf("%d", i))
 		if inst.Port != nil {
 			for _, s := range apivalidation.IsValidPortNum(int(*inst.Port)) {
 				errs = append(errs, field.Invalid(ff.Child("port"), inst.Port, s))


Test patch:
diff --git a/internal/api/v1/authproxyworkload_test.go b/internal/api/v1/authproxyworkload_test.go
index da7a6f9..61e7686 100644
--- a/internal/api/v1/authproxyworkload_test.go
+++ b/internal/api/v1/authproxyworkload_test.go
@@ -255,6 +255,16 @@ func TestAuthProxyWorkload_ValidateCreate_AuthProxyContainerSpec(t *testing.T) {
 			},
 			wantValid: false,
 		},
+		{
+			desc: "Invalid, EnableAPIs is has a bad value",
+			spec: cloudsqlapi.AuthProxyContainerSpec{
+				AdminServer: &cloudsqlapi.AdminServerSpec{
+					EnableAPIs: []string{"Nope", "Debug"},
+					Port:       wantPort,
+				},
+			},
+			wantValid: false,
+		},
 	}
 
 	for _, tc := range data {