Repository: GoogleCloudPlatform/cloud-sql-proxy-operator
Pull request #209 (2023-02-14 22:10:40)

feat: Validate AuthProxyWorkload spec.selector field
Ensure that the workload selector field is valid. Either `name` or `selector` must be set, but not both. 

Related to #36 


Patch:
diff --git a/Makefile b/Makefile
index c36de67..a7ac5d6 100644
--- a/Makefile
+++ b/Makefile
@@ -375,6 +375,23 @@ e2e_image_push: generate # Build and push a operator image to the e2e artifact r
 	DOCKER_FILE_NAME=Dockerfile-operator \
 	$(PWD)/tools/docker-build.sh
 
+###
+# Build a version of the cloud-sql-proxy from local sources
+E2E_LOCAL_PROXY_PROJECT_DIR?=/not-set
+E2E_LOCAL_PROXY_BUILD_URL_FILE=$(PWD)/bin/last-local-proxy-url.txt
+E2E_LOCAL_PROXY_BUILD_URL=$(shell cat $(E2E_LOCAL_PROXY_BUILD_URL_FILE) | tr -d "\n")
+
+.PHONY: e2e_image_push
+e2e_local_proxy_image_push: # Build and push the proxy image from a local working directory to the e2e artifact repo
+	test -d $(E2E_LOCAL_PROXY_PROJECT_DIR) && \
+	PROJECT_DIR=$(E2E_LOCAL_PROXY_PROJECT_DIR) \
+	IMAGE_NAME=cloud-sql-proxy-dev \
+	REPO_URL=$(E2E_DOCKER_URL) \
+	IMAGE_URL_OUT=$(E2E_LOCAL_PROXY_BUILD_URL_FILE) \
+	PLATFORMS=linux/amd64 \
+	DOCKER_FILE_NAME=Dockerfile \
+	$(PWD)/tools/docker-build.sh
+
 
 ##
 # Build tool dependencies
diff --git a/build.sample.env b/build.sample.env
index d968cb0..e339fc2 100644
--- a/build.sample.env
+++ b/build.sample.env
@@ -6,3 +6,9 @@
 ##
 # The gcloud project id to use for the end-to-end tests
 E2E_PROJECT_ID = my-gcloud-project
+
+##
+# To run the e2e tests using the proxy code from your local machine,
+# Uncomment this line and set the path to your local working directory
+# of the cloud-sql-proxy project.
+# E2E_LOCAL_PROXY_PROJECT_DIR = /home/me/projects/cloud-sql-proxy
diff --git a/docs/dev.md b/docs/dev.md
index 7a5b362..475b443 100644
--- a/docs/dev.md
+++ b/docs/dev.md
@@ -31,3 +31,23 @@ Then, to create the CRD for Workload
 ```
 
 
+## Running E2E tests with a custom proxy image
+
+You may want to write e2e tests for a proxy feature that has
+not been released yet. 
+
+Step 1: Check out the cloud-sql-proxy repo.
+
+Step 2: Add `E2E_LOCAL_PROXY_PROJECT_DIR = /home/me/projects/cloud-sql-proxy`
+to your `build.env`. This tells your Makefile where your proxy
+directory is. Set it to the path of your cloud-sql-proxy working directory.
+
+Step 3: Build a custom image and push it to the e2e environment
+repo. Run `make e2e_local_proxy_image_push` This will build and push
+a docker image from the proxy repo, and write the file `bin/last-local-proxy-url.txt`
+
+Step 4: Run your e2e tests. The tests will read the contents of
+the file `bin/last-local-proxy-url.txt`.
+
+Delete the file `bin/last-local-proxy-url.txt` to go back to using
+the public proxy iamge again
\ No newline at end of file
diff --git a/internal/api/v1alpha1/authproxyworkload_webhook.go b/internal/api/v1alpha1/authproxyworkload_webhook.go
index d088090..c05cafd 100644
--- a/internal/api/v1alpha1/authproxyworkload_webhook.go
+++ b/internal/api/v1alpha1/authproxyworkload_webhook.go
@@ -15,7 +15,13 @@
 package v1alpha1
 
 import (
+	"fmt"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/validation"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -30,10 +36,7 @@ func (r *AuthProxyWorkload) SetupWebhookWithManager(mgr ctrl.Manager) error {
 		Complete()
 }
 
-// TODO(user): EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
-
-//+kubebuilder:webhook:path=/mutate-cloudsql-cloud-google-com-v1alpha1-authproxyworkload,mutating=true,failurePolicy=fail,sideEffects=None,groups=cloudsql.cloud.google.com,resources=authproxyworkloads,verbs=create;update,versions=v1alpha1,name=mauthproxyworkload.kb.io,admissionReviewVersions=v1
-
+// +kubebuilder:webhook:path=/mutate-cloudsql-cloud-google-com-v1alpha1-authproxyworkload,mutating=true,failurePolicy=fail,sideEffects=None,groups=cloudsql.cloud.google.com,resources=authproxyworkloads,verbs=create;update,versions=v1alpha1,name=mauthproxyworkload.kb.io,admissionReviewVersions=v1
 var _ webhook.Defaulter = &AuthProxyWorkload{}
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type
@@ -45,31 +48,71 @@ func (r *AuthProxyWorkload) Default() {
 	}
 }
 
-// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
-//+kubebuilder:webhook:path=/validate-cloudsql-cloud-google-com-v1alpha1-authproxyworkload,mutating=false,failurePolicy=fail,sideEffects=None,groups=cloudsql.cloud.google.com,resources=authproxyworkloads,verbs=create;update,versions=v1alpha1,name=vauthproxyworkload.kb.io,admissionReviewVersions=v1
-
+// +kubebuilder:webhook:path=/validate-cloudsql-cloud-google-com-v1alpha1-authproxyworkload,mutating=false,failurePolicy=fail,sideEffects=None,groups=cloudsql.cloud.google.com,resources=authproxyworkloads,verbs=create;update,versions=v1alpha1,name=vauthproxyworkload.kb.io,admissionReviewVersions=v1
 var _ webhook.Validator = &AuthProxyWorkload{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (r *AuthProxyWorkload) ValidateCreate() error {
-	authproxyworkloadlog.Info("validate create", "name", r.Name)
-
-	// TODO(user): fill in your validation logic upon object creation.
-	return nil
+	return r.validate()
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (r *AuthProxyWorkload) ValidateUpdate(_ runtime.Object) error {
-	authproxyworkloadlog.Info("validate update", "name", r.Name)
-
-	// TODO(user): fill in your validation logic upon object update.
-	return nil
+	return r.validate()
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
 func (r *AuthProxyWorkload) ValidateDelete() error {
-	authproxyworkloadlog.Info("validate delete", "name", r.Name)
+	return nil
+}
 
-	// TODO(user): fill in your validation logic upon object deletion.
+func (r *AuthProxyWorkload) validate() error {
+	var allErrs field.ErrorList
+
+	allErrs = append(allErrs, validation.ValidateLabelName(r.Name, field.NewPath("metadata", "name"))...)
+	allErrs = append(allErrs, validateWorkload(&r.Spec.Workload, field.NewPath("spec", "workload"))...)
+
+	if len(allErrs) > 0 {
+		return apierrors.NewInvalid(
+			schema.GroupKind{
+				Group: GroupVersion.Group,
+				Kind:  "AuthProxyWorkload"},
+			r.Name, allErrs)
+	}
 	return nil
 }
+
+var supportedKinds = []string{"CronJob", "Job", "StatefulSet", "Deployment", "DaemonSet", "ReplicaSet", "Pod"}
+
+func validateWorkload(spec *WorkloadSelectorSpec, f *field.Path) field.ErrorList {
+	var errs field.ErrorList
+	if spec.Selector != nil {
+		verr := validation.ValidateLabelSelector(spec.Selector, validation.LabelSelectorValidationOptions{}, f.Child("selector"))
+		errs = append(errs, verr...)
+	}
+
+	if spec.Name != "" && spec.Selector != nil {
+		errs = append(errs, field.Invalid(f.Child("name"), spec,
+			"WorkloadSelectorSpec must specify either name or selector. Both were set."))
+	}
+	if spec.Name == "" && spec.Selector == nil {
+		errs = append(errs, field.Invalid(f.Child("name"), spec,
+			"WorkloadSelectorSpec must specify either name or selector. Neither was set."))
+	}
+
+	_, gk := schema.ParseKindArg(spec.Kind)
+	var found bool
+	for _, kind := range supportedKinds {
+		if kind == gk.Kind {
+			found = true
+			break
+		}
+	}
+	if !found {
+		errs = append(errs, field.Invalid(f.Child("kind"), spec.Kind,
+			fmt.Sprintf("Kind was %q, must be one of CronJob, Job, StatefulSet, Deployment, DaemonSet or Pod", gk.Kind)))
+
+	}
+
+	return errs
+}
diff --git a/internal/workload/podspec_updates.go b/internal/workload/podspec_updates.go
index c815a44..30fc942 100644
--- a/internal/workload/podspec_updates.go
+++ b/internal/workload/podspec_updates.go
@@ -204,38 +204,30 @@ func (u *Updater) ConfigureWorkload(wl *PodWorkload, matches []*cloudsqlapi.Auth
 }
 
 type managedEnvVar struct {
-	Instance             dbInstance        `json:"dbInstance"`
-	OperatorManagedValue corev1.EnvVar     `json:"operatorManagedValue"`
-	OriginalValues       map[string]string `json:"originalValues,omitempty"`
+	Instance             proxyInstanceID `json:"proxyInstanceID"`
+	ContainerName        string          `json:"containerName"`
+	OperatorManagedValue corev1.EnvVar   `json:"operatorManagedValue"`
 }
 
 type managedPort struct {
-	Instance       dbInstance       `json:"dbInstance"`
-	OriginalValues map[string]int32 `json:"originalValues,omitempty"`
-	Port           int32            `json:"port,omitempty"`
+	Instance proxyInstanceID `json:"proxyInstanceID"`
+	Port     int32           `json:"port,omitempty"`
 }
 
 type managedVolume struct {
 	Volume      corev1.Volume      `json:"volume"`
 	VolumeMount corev1.VolumeMount `json:"volumeMount"`
-	Instance    dbInstance         `json:"dbInstance"`
+	Instance    proxyInstanceID    `json:"proxyInstanceID"`
 }
 
-type dbInstance struct {
+// proxyInstanceID is an identifier for a proxy and/or specific proxy database
+// instance that created the EnvVar or Port. When this is empty, means that the
+// EnvVar or Port was created by the user, and is not associated with a proxy
+type proxyInstanceID struct {
 	AuthProxyWorkload types.NamespacedName `json:"authProxyWorkload"`
 	ConnectionString  string               `json:"connectionString"`
 }
 
-func dbInst(namespace, name, connectionString string) dbInstance {
-	return dbInstance{
-		AuthProxyWorkload: types.NamespacedName{
-			Namespace: namespace,
-			Name:      name,
-		},
-		ConnectionString: connectionString,
-	}
-}
-
 // updateState holds internal state while a particular workload being configured
 // with one or more DBInstances.
 type updateState struct {
@@ -248,14 +240,22 @@ type updateState struct {
 // workloadMods holds all modifications to this workload done by the operator so
 // so that it can be undone later.
 type workloadMods struct {
-	DBInstances  []*dbInstance    `json:"dbInstances"`
-	EnvVars      []*managedEnvVar `json:"envVars"`
-	VolumeMounts []*managedVolume `json:"volumeMounts"`
-	Ports        []*managedPort   `json:"ports"`
+	DBInstances  []*proxyInstanceID `json:"dbInstances"`
+	EnvVars      []*managedEnvVar   `json:"envVars"`
+	VolumeMounts []*managedVolume   `json:"volumeMounts"`
+	Ports        []*managedPort     `json:"ports"`
 }
 
-func (s *updateState) addInUsePort(p int32, containerName string) {
-	s.addPort(p, containerName, types.NamespacedName{}, "")
+func (s *updateState) addWorkloadPort(p int32) {
+	// This port is associated with the workload, not the proxy.
+	// so this uses an empty proxyInstanceID{}
+	s.addPort(p, proxyInstanceID{})
+}
+
+func (s *updateState) addProxyPort(p int32) {
+	// This port is associated with the workload, not the proxy.
+	// so this uses an empty proxyInstanceID{}
+	s.addPort(p, proxyInstanceID{})
 }
 
 // isPortInUse checks if the port is in use.
@@ -314,12 +314,18 @@ func (s *updateState) useInstancePort(p *cloudsqlapi.AuthProxyWorkload, is *clou
 				port, is.ConnectionString), p)
 	}
 
-	s.addPort(port, "", n, is.ConnectionString)
+	s.addPort(port, proxyInstanceID{
+		AuthProxyWorkload: types.NamespacedName{
+			Name:      p.Name,
+			Namespace: p.Namespace,
+		},
+		ConnectionString: is.ConnectionString,
+	})
 
 	return port
 }
 
-func (s *updateState) addPort(p int32, containerName string, n types.NamespacedName, connectionString string) {
+func (s *updateState) addPort(p int32, instance proxyInstanceID) {
 	var mp *managedPort
 
 	for i := 0; i < len(s.mods.Ports); i++ {
@@ -330,46 +336,78 @@ func (s *updateState) addPort(p int32, containerName string, n types.NamespacedN
 
 	if mp == nil {
 		mp = &managedPort{
-			Instance:       dbInst(n.Namespace, n.Name, connectionString),
-			Port:           p,
-			OriginalValues: map[string]int32{},
+			Instance: instance,
+			Port:     p,
 		}
 		s.mods.Ports = append(s.mods.Ports, mp)
 	}
-	if containerName != "" && !strings.HasPrefix(containerName, ContainerPrefix) {
-		mp.OriginalValues[containerName] = p
-	}
+}
 
+func (s *updateState) addProxyContainerEnvVar(p *cloudsqlapi.AuthProxyWorkload, k, v string) {
+	s.addEnvVar(p, managedEnvVar{
+		Instance: proxyInstanceID{
+			AuthProxyWorkload: types.NamespacedName{
+				Namespace: p.Namespace,
+				Name:      p.Name,
+			},
+		},
+		ContainerName:        ContainerName(p),
+		OperatorManagedValue: corev1.EnvVar{Name: k, Value: v},
+	})
 }
 
 // addWorkloadEnvVar adds or replaces the envVar based on its Name, returning the old and new values
 func (s *updateState) addWorkloadEnvVar(p *cloudsqlapi.AuthProxyWorkload, is *cloudsqlapi.InstanceSpec, ev corev1.EnvVar) {
+	s.addEnvVar(p, managedEnvVar{
+		Instance: proxyInstanceID{
+			AuthProxyWorkload: types.NamespacedName{
+				Namespace: p.Namespace,
+				Name:      p.Name,
+			},
+			ConnectionString: is.ConnectionString,
+		},
+		OperatorManagedValue: ev,
+	})
+}
+func (s *updateState) addEnvVar(p *cloudsqlapi.AuthProxyWorkload, v managedEnvVar) {
 	for i := 0; i < len(s.mods.EnvVars); i++ {
-		if s.mods.EnvVars[i].OperatorManagedValue.Name == ev.Name {
-			old := s.mods.EnvVars[i].OperatorManagedValue
-			s.mods.EnvVars[i].OperatorManagedValue = ev
-			if old.Value != ev.Value {
-				s.addError(cloudsqlapi.ErrorCodeEnvConflict,
-					fmt.Sprintf("environment variable named %s already exists", ev.Name), p)
-			}
+		oldEnv := s.mods.EnvVars[i]
+		// if the values don't match and either one is global, or its set twice
+		if isEnvVarConflict(oldEnv, v) {
+			s.addError(cloudsqlapi.ErrorCodeEnvConflict,
+				fmt.Sprintf("environment variable named %s is set more than once",
+					oldEnv.OperatorManagedValue.Name),
+				p)
 			return
 		}
 	}
-	s.mods.EnvVars = append(s.mods.EnvVars, &managedEnvVar{
-		Instance:             dbInst(p.Namespace, p.Name, is.ConnectionString),
-		OriginalValues:       map[string]string{},
-		OperatorManagedValue: ev,
-	})
+
+	s.mods.EnvVars = append(s.mods.EnvVars, &v)
+}
+
+func isEnvVarConflict(oldEnv *managedEnvVar, v managedEnvVar) bool {
+	// it's a different name, no conflict
+	if oldEnv.OperatorManagedValue.Name != v.OperatorManagedValue.Name {
+		return false
+	}
+
+	// if the envvar is intended for a different container
+	if oldEnv.ContainerName != v.ContainerName && oldEnv.ContainerName != "" && v.ContainerName != "" {
+		return false
+	}
+
+	// different value, therefore conflict
+	return oldEnv.OperatorManagedValue.Value != v.OperatorManagedValue.Value
 }
 
 func (s *updateState) initState(pl []*cloudsqlapi.AuthProxyWorkload) {
 	// Reset the mods.DBInstances to the list of pl being
 	// applied right now.
-	s.mods.DBInstances = make([]*dbInstance, 0, len(pl))
+	s.mods.DBInstances = make([]*proxyInstanceID, 0, len(pl))
 	for _, wl := range pl {
 		for _, instance := range wl.Spec.Instances {
 			s.mods.DBInstances = append(s.mods.DBInstances,
-				&dbInstance{
+				&proxyInstanceID{
 					AuthProxyWorkload: types.NamespacedName{
 						Namespace: wl.Namespace,
 						Name:      wl.Name,
@@ -399,7 +437,7 @@ func (s *updateState) update(wl *PodWorkload, matches []*cloudsqlapi.AuthProxyWo
 	for i := 0; i < len(nonAuthProxyContainers); i++ {
 		c := nonAuthProxyContainers[i]
 		for j := 0; j < len(c.Ports); j++ {
-			s.addInUsePort(c.Ports[j].ContainerPort, c.Name)
+			s.addWorkloadPort(c.Ports[j].ContainerPort)
 		}
 	}
 
@@ -455,19 +493,22 @@ func (s *updateState) updateContainer(p *cloudsqlapi.AuthProxyWorkload, wl Workl
 		return
 	}
 
-	// Build the c
-	var cliArgs []string
-
 	// always enable http port healthchecks on 0.0.0.0 and structured logs
-	cliArgs = s.addHealthCheck(p, c, cliArgs)
+	s.addHealthCheck(p, c)
 
 	// add the user agent
-	cliArgs = append(cliArgs, fmt.Sprintf("--user-agent=%v", s.updater.userAgent))
+	s.addProxyContainerEnvVar(p, "CSQL_PROXY_USER_AGENT", s.updater.userAgent)
+
+	// configure structured logs
+	s.addProxyContainerEnvVar(p, "CSQL_PROXY_STRUCTURED_LOGS", "true")
 
 	c.Name = ContainerName(p)
 	c.ImagePullPolicy = "IfNotPresent"
 
-	cliArgs = s.applyContainerSpec(p, c, cliArgs)
+	s.applyContainerSpec(p, c)
+
+	// Build the c
+	var cliArgs []string
 
 	// Instances
 	for i := range p.Spec.Instances {
@@ -527,12 +568,12 @@ func (s *updateState) updateContainer(p *cloudsqlapi.AuthProxyWorkload, wl Workl
 
 // applyContainerSpec applies settings from cloudsqlapi.AuthProxyContainerSpec
 // to the container
-func (s *updateState) applyContainerSpec(p *cloudsqlapi.AuthProxyWorkload, c *corev1.Container, cliArgs []string) []string {
+func (s *updateState) applyContainerSpec(p *cloudsqlapi.AuthProxyWorkload, c *corev1.Container) {
 	c.Image = s.defaultProxyImage()
 	c.Resources = defaultContainerResources
 
 	if p.Spec.AuthProxyContainer == nil {
-		return cliArgs
+		return
 	}
 
 	if p.Spec.AuthProxyContainer.Image != "" {
@@ -544,25 +585,32 @@ func (s *updateState) applyContainerSpec(p *cloudsqlapi.AuthProxyWorkload, c *co
 	}
 
 	if p.Spec.AuthProxyContainer.SQLAdminAPIEndpoint != "" {
-		cliArgs = append(cliArgs, "--sqladmin-api-endpoint="+p.Spec.AuthProxyContainer.SQLAdminAPIEndpoint)
+		s.addProxyContainerEnvVar(p, "CSQL_PROXY_SQLADMIN_API_ENDPOINT", p.Spec.AuthProxyContainer.SQLAdminAPIEndpoint)
 	}
 	if p.Spec.AuthProxyContainer.MaxConnections != nil &&
 		*p.Spec.AuthProxyContainer.MaxConnections != 0 {
-		cliArgs = append(cliArgs, fmt.Sprintf("--max-connections=%d", *p.Spec.AuthProxyContainer.MaxConnections))
+		s.addProxyContainerEnvVar(p, "CSQL_PROXY_MAX_CONNECTIONS", fmt.Sprintf("%d", *p.Spec.AuthProxyContainer.MaxConnections))
 	}
 	if p.Spec.AuthProxyContainer.MaxSigtermDelay != nil &&
 		*p.Spec.AuthProxyContainer.MaxSigtermDelay != 0 {
-		cliArgs = append(cliArgs, fmt.Sprintf("--max-sigterm-delay=%d", *p.Spec.AuthProxyContainer.MaxSigtermDelay))
+		s.addProxyContainerEnvVar(p, "CSQL_PROXY_MAX_SIGTERM_DELAY", fmt.Sprintf("%d", *p.Spec.AuthProxyContainer.MaxSigtermDelay))
 	}
 
-	return cliArgs
+	return
 }
 
 // updateContainerEnv applies global container state to all containers
 func (s *updateState) updateContainerEnv(c *corev1.Container) {
 	for i := 0; i < len(s.mods.EnvVars); i++ {
 		var found bool
-		operatorEnv := s.mods.EnvVars[i].OperatorManagedValue
+		v := s.mods.EnvVars[i]
+		operatorEnv := v.OperatorManagedValue
+
+		// If this EnvVar is not for this container and not for all containers
+		// don't add it to this container.
+		if v.ContainerName != c.Name && v.ContainerName != "" {
+			continue
+		}
 
 		for j := 0; j < len(c.Env); j++ {
 			if operatorEnv.Name == c.Env[j].Name {
@@ -578,11 +626,8 @@ func (s *updateState) updateContainerEnv(c *corev1.Container) {
 }
 
 // addHealthCheck adds the health check declaration to this workload.
-func (s *updateState) addHealthCheck(_ *cloudsqlapi.AuthProxyWorkload, c *corev1.Container, cliArgs []string) []string {
+func (s *updateState) addHealthCheck(p *cloudsqlapi.AuthProxyWorkload, c *corev1.Container) {
 	port := DefaultHealthCheckPort
-	for s.isPortInUse(port) {
-		port++
-	}
 
 	c.StartupProbe = &corev1.Probe{
 		ProbeHandler: corev1.ProbeHandler{HTTPGet: &corev1.HTTPGetAction{
@@ -605,12 +650,12 @@ func (s *updateState) addHealthCheck(_ *cloudsqlapi.AuthProxyWorkload, c *corev1
 		}},
 		PeriodSeconds: 30,
 	}
-	cliArgs = append(cliArgs,
-		fmt.Sprintf("--http-port=%d", port),
-		"--http-address=0.0.0.0",
-		"--health-check",
-		"--structured-logs")
-	return cliArgs
+	// Add a port that is associated with the proxy, but not a specific db instance
+	s.addPort(port, proxyInstanceID{AuthProxyWorkload: types.NamespacedName{Namespace: p.Namespace, Name: p.Name}})
+	s.addProxyContainerEnvVar(p, "CSQL_PROXY_HTTP_PORT", fmt.Sprintf("%d", port))
+	s.addProxyContainerEnvVar(p, "CSQL_PROXY_HTTP_ADDRESS", "0.0.0.0")
+	s.addProxyContainerEnvVar(p, "CSQL_PROXY_HEALTH_CHECK", "true")
+	return
 }
 
 func (s *updateState) addError(errorCode, description string, p *cloudsqlapi.AuthProxyWorkload) {


Test patch:
diff --git a/internal/api/v1alpha1/authproxyworkload_test.go b/internal/api/v1alpha1/authproxyworkload_test.go
new file mode 100644
index 0000000..91faef8
--- /dev/null
+++ b/internal/api/v1alpha1/authproxyworkload_test.go
@@ -0,0 +1,185 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1alpha1_test
+
+import (
+	"testing"
+
+	cloudsqlapi "github.com/GoogleCloudPlatform/cloud-sql-proxy-operator/internal/api/v1alpha1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestAuthProxyWorkload_ValidateCreate(t *testing.T) {
+	data := []struct {
+		desc      string
+		spec      cloudsqlapi.AuthProxyWorkloadSpec
+		wantValid bool
+	}{
+		{
+			desc: "Valid WorkloadSelectorSpec with Name",
+			spec: cloudsqlapi.AuthProxyWorkloadSpec{
+				Workload: cloudsqlapi.WorkloadSelectorSpec{
+					Kind: "Deployment",
+					Name: "webapp",
+				},
+			},
+			wantValid: true,
+		},
+		{
+			desc: "Valid WorkloadSelectorSpec with Selector",
+			spec: cloudsqlapi.AuthProxyWorkloadSpec{
+				Workload: cloudsqlapi.WorkloadSelectorSpec{
+					Kind: "Deployment",
+					Selector: &v1.LabelSelector{
+						MatchLabels: map[string]string{"app": "sample"},
+					},
+				},
+			},
+			wantValid: true,
+		},
+		{
+			desc: "Invalid, both workload selector and name both set",
+			spec: cloudsqlapi.AuthProxyWorkloadSpec{
+				Workload: cloudsqlapi.WorkloadSelectorSpec{
+					Kind: "Deployment",
+					Name: "webapp",
+					Selector: &v1.LabelSelector{
+						MatchLabels: map[string]string{"app": "sample"},
+					},
+				},
+			},
+			wantValid: false,
+		},
+		{
+			desc: "Invalid, WorkloadSelector missing name and selector",
+			spec: cloudsqlapi.AuthProxyWorkloadSpec{
+				Workload: cloudsqlapi.WorkloadSelectorSpec{Kind: "Deployment"},
+			},
+			wantValid: false,
+		},
+		{
+			desc: "Valid, Instance configured with PortEnvName",
+			spec: cloudsqlapi.AuthProxyWorkloadSpec{
+				Workload: cloudsqlapi.WorkloadSelectorSpec{
+					Kind: "Deployment",
+					Name: "webapp",
+				},
+				Instances: []cloudsqlapi.InstanceSpec{{
+					ConnectionString: "proj:region:db2",
+					PortEnvName:      "DB_PORT",
+				}},
+			},
+			wantValid: true,
+		},
+	}
+
+	for _, tc := range data {
+		t.Run(tc.desc, func(t *testing.T) {
+			p := cloudsqlapi.AuthProxyWorkload{
+				ObjectMeta: v1.ObjectMeta{Name: "sample"},
+				Spec:       tc.spec,
+			}
+			err := p.ValidateCreate()
+			gotValid := err == nil
+			switch {
+			case tc.wantValid && !gotValid:
+				t.Errorf("wants create valid, got error %v", err)
+				printFieldErrors(t, err)
+			case !tc.wantValid && gotValid:
+				t.Errorf("wants an error on create, got no error")
+			default:
+				t.Logf("create passed %s", tc.desc)
+				// test passes, do nothing.
+			}
+		})
+	}
+}
+
+func TestAuthProxyWorkload_ValidateUpdate(t *testing.T) {
+	data := []struct {
+		desc      string
+		spec      cloudsqlapi.AuthProxyWorkloadSpec
+		oldSpec   cloudsqlapi.AuthProxyWorkloadSpec
+		wantValid bool
+	}{
+		{
+			desc: "Valid, update adds another instance",
+			spec: cloudsqlapi.AuthProxyWorkloadSpec{
+				Workload: cloudsqlapi.WorkloadSelectorSpec{
+					Kind: "Deployment",
+					Name: "webapp",
+				},
+				Instances: []cloudsqlapi.InstanceSpec{{
+					ConnectionString: "proj:region:db1",
+					PortEnvName:      "DB_PORT",
+				}},
+			},
+			oldSpec: cloudsqlapi.AuthProxyWorkloadSpec{
+				Workload: cloudsqlapi.WorkloadSelectorSpec{
+					Kind: "Deployment",
+					Name: "webapp",
+				},
+				Instances: []cloudsqlapi.InstanceSpec{
+					{
+						ConnectionString: "proj:region:db1",
+						PortEnvName:      "DB_PORT",
+					},
+					{
+						ConnectionString: "proj:region:db2",
+						PortEnvName:      "DB_PORT2",
+					},
+				},
+			},
+			wantValid: true,
+		},
+	}
+
+	for _, tc := range data {
+		t.Run(tc.desc, func(t *testing.T) {
+			p := cloudsqlapi.AuthProxyWorkload{
+				ObjectMeta: v1.ObjectMeta{Name: "sample"},
+				Spec:       tc.spec,
+			}
+			oldP := cloudsqlapi.AuthProxyWorkload{
+				ObjectMeta: v1.ObjectMeta{Name: "sample"},
+				Spec:       tc.oldSpec}
+
+			err := p.ValidateUpdate(&oldP)
+			gotValid := err == nil
+
+			switch {
+			case tc.wantValid && !gotValid:
+				t.Errorf("wants create valid, got error %v", err)
+			case !tc.wantValid && gotValid:
+				t.Errorf("wants an error on create, got no error")
+			default:
+				t.Logf("update passed %s", tc.desc)
+				// test passes, do nothing.
+			}
+		})
+	}
+}
+
+func printFieldErrors(t *testing.T, err error) {
+	t.Helper()
+	statusErr, ok := err.(*apierrors.StatusError)
+	if ok {
+		t.Errorf("Field status errors: ")
+		for _, v := range statusErr.Status().Details.Causes {
+			t.Errorf("   %v %v: %v ", v.Field, v.Type, v.Message)
+		}
+	}
+}
diff --git a/internal/workload/podspec_updates_test.go b/internal/workload/podspec_updates_test.go
index 37d0ac8..a4b7dd0 100644
--- a/internal/workload/podspec_updates_test.go
+++ b/internal/workload/podspec_updates_test.go
@@ -517,6 +517,7 @@ func TestProxyCLIArgs(t *testing.T) {
 		proxySpec            v1alpha1.AuthProxyWorkloadSpec
 		wantProxyArgContains []string
 		wantErrorCodes       []string
+		wantWorkloadEnv      map[string]string
 	}
 	wantTrue := true
 	wantFalse := false
@@ -533,12 +534,12 @@ func TestProxyCLIArgs(t *testing.T) {
 					PortEnvName:      "DB_PORT",
 				}},
 			},
-			wantProxyArgContains: []string{
-				"--structured-logs",
-				"--health-check",
-				fmt.Sprintf("--http-port=%d", workload.DefaultHealthCheckPort),
-				"--http-address=0.0.0.0",
-				"--user-agent=cloud-sql-proxy-operator/dev",
+			wantWorkloadEnv: map[string]string{
+				"CSQL_PROXY_STRUCTURED_LOGS": "true",
+				"CSQL_PROXY_HEALTH_CHECK":    "true",
+				"CSQL_PROXY_HTTP_PORT":       fmt.Sprintf("%d", workload.DefaultHealthCheckPort),
+				"CSQL_PROXY_HTTP_ADDRESS":    "0.0.0.0",
+				"CSQL_PROXY_USER_AGENT":      "cloud-sql-proxy-operator/dev",
 			},
 		},
 		{
@@ -621,25 +622,6 @@ func TestProxyCLIArgs(t *testing.T) {
 				fmt.Sprintf("hello:world:one?port=%d&private-ip=true", workload.DefaultFirstPort),
 				fmt.Sprintf("hello:world:two?port=%d&private-ip=false", workload.DefaultFirstPort+1)},
 		},
-		{
-			desc: "global flags",
-			proxySpec: v1alpha1.AuthProxyWorkloadSpec{
-				AuthProxyContainer: &v1alpha1.AuthProxyContainerSpec{
-					SQLAdminAPIEndpoint: "https://example.com",
-					MaxConnections:      ptr(int64(10)),
-					MaxSigtermDelay:     ptr(int64(20)),
-				},
-				Instances: []v1alpha1.InstanceSpec{{
-					ConnectionString: "hello:world:one",
-				}},
-			},
-			wantProxyArgContains: []string{
-				fmt.Sprintf("hello:world:one?port=%d", workload.DefaultFirstPort),
-				"--sqladmin-api-endpoint=https://example.com",
-				"--max-connections=10",
-				"--max-sigterm-delay=20",
-			},
-		},
 		{
 			desc: "port conflict with other instance causes error",
 			proxySpec: v1alpha1.AuthProxyWorkloadSpec{
@@ -714,6 +696,19 @@ func TestProxyCLIArgs(t *testing.T) {
 			// test that port cli args are set correctly
 			assertContainerArgsContains(t, csqlContainer.Args, tc.wantProxyArgContains)
 
+			// Test that workload has the right env vars
+			for wantKey, wantValue := range tc.wantWorkloadEnv {
+				gotEnvVar, err := findEnvVar(wl, csqlContainer.Name, wantKey)
+				if err != nil {
+					t.Error(err)
+					continue
+				}
+
+				if gotEnvVar.Value != wantValue {
+					t.Errorf("got %v, wants %v workload env var %v", gotEnvVar, wantValue, wantKey)
+				}
+			}
+
 		})
 	}
 
diff --git a/tests/setup_test.go b/tests/setup_test.go
index 9244c95..5b86bf4 100644
--- a/tests/setup_test.go
+++ b/tests/setup_test.go
@@ -110,7 +110,7 @@ func setupTests() (func(), error) {
 	}
 
 	// Read e2e test configuration
-	proxyImageURL = loadValue("PROXY_IMAGE_URL", "../bin/last-proxy-image-url.txt", workload.DefaultProxyImage)
+	proxyImageURL = loadValue("PROXY_IMAGE_URL", "../bin/last-local-proxy-url.txt", workload.DefaultProxyImage)
 	operatorURL = loadValue("OPERATOR_IMAGE_URL", "../bin/last-gcloud-operator-url.txt", "operator:latest")
 	testInfraPath := loadValue("TEST_INFRA_JSON", "", "../bin/testinfra.json")
 	ti, err := loadTestInfra(testInfraPath)