upgrade operator SDK from v1.28 to v1.33 (#33)

* upgrade operator SDK from v1.28 to v1.33

* pre-merge

* back to control gen v0.9.2

* Move go/v4 plugin from alpha to stable

Author: yonathan-shtekel

.run/Build & Run.run.xml

@@ -7,9 +7,9 @@
       <env name="ENABLE_WEBHOOKS" value="false" />
     </envs>
     <kind value="FILE" />
-    <package value="$PROJECT_DIR$/main.go" />
+    <package value="$PROJECT_DIR$/cmd/main.go" />
     <directory value="$PROJECT_DIR$" />
-    <filePath value="$PROJECT_DIR$/main.go" />
+    <filePath value="$PROJECT_DIR$/cmd/main.go" />
     <method v="2" />
   </configuration>
 </component>

.run/Test v1alpha1 (api_v1alpha1).run.xml

@@ -1,8 +1,8 @@
-golang 1.19.8
+golang 1.21.6
 act 0.2.26
 kuttl 0.15.0
-operator-sdk 1.28.0
+operator-sdk 1.33.0
 kind 0.18.0
 istioctl 1.17.2
-golangci-lint 1.53.2
-yq 4.40.5
+golangci-lint 1.54.1
+yq 4.40.5

.tool-versions

@@ -1,5 +1,5 @@
 # Build the manager binary
-FROM golang:1.19 as builder
+FROM golang:1.21 as builder
 
 WORKDIR /workspace
 # Copy the Go Modules manifests
@@ -10,14 +10,14 @@ COPY go.sum go.sum
 RUN go mod download
 
 # Copy the go source
-COPY main.go main.go
+COPY cmd/main.go cmd/main.go
 COPY api/ api/
-COPY controllers/ controllers/
+COPY internal/controller/ internal/controller/
 COPY pkg/ pkg/
 COPY extensions/ extensions/
 
 # Build
-RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -a -o manager main.go
+RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -a -o manager cmd/main.go
 
 # Use distroless as minimal base image to package the manager binary
 # Refer to https://github.com/GoogleContainerTools/distroless for more details

Dockerfile

@@ -117,11 +117,11 @@ lint: ## Run various linters
 
 .PHONY: build
 build: manifests generate fmt vet ## Build manager binary.
-	go build -o bin/manager main.go
+	go build -o bin/manager cmd/main.go
 
 .PHONY: run
 run: manifests generate fmt vet ## Run a controller from your host.
-	ENABLE_WEBHOOKS=false go run ./main.go -zap-devel
+	ENABLE_WEBHOOKS=false go run ./cmd/main.go -zap-devel
 
 .PHONY: docker-build
 docker-build: ## Build docker image with the manager.
@@ -215,7 +215,8 @@ $(CONTROLLER_GEN): $(LOCALBIN)
 .PHONY: envtest
 envtest: $(ENVTEST) ## Download envtest-setup locally if necessary.
 $(ENVTEST): $(LOCALBIN)
-	test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
+	# https://github.com/kubernetes-sigs/kubebuilder/issues/2480
+	test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/[email protected]
 
 .PHONY: bundle
 bundle: manifests kustomize ## Generate bundle manifests and metadata, then validate generated files.

Makefile

@@ -1,6 +1,6 @@
 domain: com
 layout:
-- go.kubebuilder.io/v3
+- go.kubebuilder.io/v4
 plugins:
   manifests.sdk.operatorframework.io/v2: {}
   scorecard.sdk.operatorframework.io/v2: {}

PROJECT

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"k8s.io/utils/strings/slices"
 	"reflect"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -44,37 +45,37 @@ func (de *DynamicEnv) SetupWebhookWithManager(mgr ctrl.Manager) error {
 var _ webhook.Validator = &DynamicEnv{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
-func (de *DynamicEnv) ValidateCreate() error {
+func (de *DynamicEnv) ValidateCreate() (admission.Warnings, error) {
 	dynamicenvlog.Info("validate create", "name", de.Name)
 
 	if err := de.validateIstioMatchAnyOf(); err != nil {
-		return err
+		return nil, err
 	}
 	if err := de.validateSubsetsProperties(); err != nil {
-		return err
+		return nil, err
 	}
 
-	return de.validateStringMatchOneOf()
+	return nil, de.validateStringMatchOneOf()
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
-func (de *DynamicEnv) ValidateUpdate(old runtime.Object) error {
+func (de *DynamicEnv) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
 	dynamicenvlog.Info("validate update", "name", de.Name)
 
 	if err := de.validateIstioMatchImmutable(old); err != nil {
-		return err
+		return nil, err
 	}
 	if err := de.validateSubsetsProperties(); err != nil {
-		return err
+		return nil, err
 	}
-	return de.validatePartialUpdateSubsets(old)
+	return nil, de.validatePartialUpdateSubsets(old)
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
-func (de *DynamicEnv) ValidateDelete() error {
+func (de *DynamicEnv) ValidateDelete() (admission.Warnings, error) {
 	dynamicenvlog.Info("validate delete", "name", de.Name)
 
-	return nil
+	return nil, nil
 }
 
 // validateStringMatchOneOf must validate one and only one schema (oneOf) of StringMatch is defined

api/v1alpha1/dynamicenv_webhook.go

@@ -85,7 +85,7 @@ var _ = Describe("Validating Webhook", func() {
 
 		It("Does not allow to update matchers", func() {
 			old := runtime.Object(&base)
-			err := updated.ValidateUpdate(old)
+			_, err := updated.ValidateUpdate(old)
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("field is immutable"))
 		})
@@ -148,7 +148,7 @@ var _ = Describe("Validating Webhook", func() {
 
 			for ind, item := range testCases {
 				By(fmt.Sprintf("multi stringMatch %d", ind))
-				err := item.ValidateCreate()
+				_, err := item.ValidateCreate()
 				Expect(err).To(HaveOccurred())
 				Expect(err.Error()).To(ContainSubstring("oneOf"))
 			}
@@ -163,7 +163,7 @@ var _ = Describe("Validating Webhook", func() {
 				},
 				Spec: DynamicEnvSpec{},
 			}
-			err := noMatchDe.ValidateCreate()
+			_, err := noMatchDe.ValidateCreate()
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("empty IstioMatch"))
 		})
@@ -192,7 +192,7 @@ var _ = Describe("Validating Webhook", func() {
 				},
 			}
 
-			err := de.ValidateCreate()
+			_, err := de.ValidateCreate()
 			Expect(err).To(BeNil())
 		})
 
@@ -263,9 +263,9 @@ var _ = Describe("Validating Webhook", func() {
 				},
 			}
 
-			err1 := de1.ValidateCreate()
+			_, err1 := de1.ValidateCreate()
 			Expect(err1).To(BeNil(), "Should accept number of replicas in subsets")
-			err2 := de2.ValidateCreate()
+			_, err2 := de2.ValidateCreate()
 			Expect(err2).To(BeNil(), "Should accept number of replicas in consumers")
 		})
 
@@ -274,7 +274,7 @@ var _ = Describe("Validating Webhook", func() {
 			if err != nil {
 				Fail(err.Error())
 			}
-			resultError := de.ValidateCreate()
+			_, resultError := de.ValidateCreate()
 			Expect(resultError).To(HaveOccurred())
 			Expect(resultError.Error()).To(ContainSubstring("At least a single container or init-container"))
 		})
@@ -284,14 +284,14 @@ var _ = Describe("Validating Webhook", func() {
 			if err != nil {
 				Fail(err.Error())
 			}
-			resultError := de.ValidateCreate()
+			_, resultError := de.ValidateCreate()
 			Expect(resultError).To(HaveOccurred())
 			Expect(resultError.Error()).To(ContainSubstring("names are unique"))
 		})
 
 		DescribeTable("Create rejects invalid subset properties",
 			func(de *DynamicEnv, errMsg string) {
-				err := de.ValidateCreate()
+				_, err := de.ValidateCreate()
 				Expect(err).To(Not(BeNil()))
 				Expect(err.Error()).To(ContainSubstring(errMsg))
 			},
@@ -517,7 +517,7 @@ var _ = Describe("Validating Webhook", func() {
 					},
 				}
 
-				err := de.ValidateCreate()
+				_, err := de.ValidateCreate()
 				Expect(err).To(HaveOccurred())
 				Expect(err.Error()).To(ContainSubstring("Invalid value"))
 			},
@@ -548,7 +548,7 @@ var _ = Describe("Validating Webhook", func() {
 				if err != nil {
 					Fail("Error decoding currentData")
 				}
-				errorResult := current.ValidateUpdate(runtime.Object(&old))
+				_, errorResult := current.ValidateUpdate(runtime.Object(&old))
 				Expect(errorResult).To(BeNil())
 			},
 			Entry(
@@ -579,7 +579,7 @@ var _ = Describe("Validating Webhook", func() {
 				if err != nil {
 					Fail("Error decoding currentData")
 				}
-				errorResult := current.ValidateUpdate(runtime.Object(&old))
+				_, errorResult := current.ValidateUpdate(runtime.Object(&old))
 				Expect(errorResult).To(HaveOccurred())
 				Expect(errorResult.Error()).To(ContainSubstring(partialError))
 			},

api/v1alpha1/dynamicenv_webhook_test.go

@@ -23,15 +23,18 @@ import (
 	"k8s.io/client-go/rest"
 	"net"
 	"path/filepath"
+	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"testing"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
-	admissionv1beta1 "k8s.io/api/admission/v1beta1"
+	admissionv1 "k8s.io/api/admission/v1"
 	//+kubebuilder:scaffold:imports
 	"k8s.io/apimachinery/pkg/runtime"
+	_ "k8s.io/client-go/plugin/pkg/client/auth/oidc"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
@@ -58,7 +61,6 @@ var _ = BeforeSuite(func() {
 	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
 
 	ctx, cancel = context.WithCancel(context.TODO())
-
 	By("bootstrapping test environment")
 	testEnv = &envtest.Environment{
 		CRDDirectoryPaths:     []string{filepath.Join("..", "..", "config", "crd", "bases")},
@@ -77,7 +79,7 @@ var _ = BeforeSuite(func() {
 	err = AddToScheme(scheme)
 	Expect(err).NotTo(HaveOccurred())
 
-	err = admissionv1beta1.AddToScheme(scheme)
+	err = admissionv1.AddToScheme(scheme)
 	Expect(err).NotTo(HaveOccurred())
 
 	//+kubebuilder:scaffold:scheme
@@ -88,14 +90,20 @@ var _ = BeforeSuite(func() {
 
 	// start webhook server using Manager
 	webhookInstallOptions := &testEnv.WebhookInstallOptions
+
 	mgr, err := ctrl.NewManager(cfg, ctrl.Options{
-		Scheme:             scheme,
-		Host:               webhookInstallOptions.LocalServingHost,
-		Port:               webhookInstallOptions.LocalServingPort,
-		CertDir:            webhookInstallOptions.LocalServingCertDir,
-		LeaderElection:     false,
-		MetricsBindAddress: "0",
+		Scheme: scheme,
+		WebhookServer: webhook.NewServer(webhook.Options{
+			Host:    webhookInstallOptions.LocalServingHost,
+			Port:    webhookInstallOptions.LocalServingPort,
+			CertDir: webhookInstallOptions.LocalServingCertDir,
+		}),
+		LeaderElection: false,
+		Metrics: server.Options{
+			BindAddress: "0",
+		},
 	})
+
 	Expect(err).NotTo(HaveOccurred())
 
 	err = (&DynamicEnv{}).SetupWebhookWithManager(mgr)

api/v1alpha1/webhook_suite_test.go

@@ -19,9 +19,12 @@ package main
 import (
 	"flag"
 	"fmt"
+	"github.com/riskified/dynamic-environment/internal/controller"
 	"github.com/riskified/dynamic-environment/pkg/metrics"
 	"github.com/riskified/dynamic-environment/pkg/names"
 	"os"
+	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"strings"
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
@@ -37,7 +40,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	riskifiedv1alpha1 "github.com/riskified/dynamic-environment/api/v1alpha1"
-	"github.com/riskified/dynamic-environment/controllers"
 	// "istio.io/client-go/pkg/apis/networking/v1alpha3"
 	//+kubebuilder:scaffold:imports
 )
@@ -105,19 +107,24 @@ func main() {
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
 
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
-		Scheme:                 scheme,
-		MetricsBindAddress:     metricsAddr,
-		Port:                   9443,
+		Scheme: scheme,
+		Metrics: server.Options{
+			BindAddress: metricsAddr,
+		},
+		WebhookServer: webhook.NewServer(webhook.Options{
+			Port: 9443,
+		}),
 		HealthProbeBindAddress: probeAddr,
 		LeaderElection:         enableLeaderElection,
 		LeaderElectionID:       "dynamic-environment-lock",
 	})
+
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")
 		os.Exit(1)
 	}
 
-	if err = (&controllers.DynamicEnvReconciler{
+	if err = (&controller.DynamicEnvReconciler{
 		Client:         mgr.GetClient(),
 		Scheme:         mgr.GetScheme(),
 		VersionLabel:   versionLabel,