Merge pull request #132 from nais/client-go-strict-field-validation

deployd: use strict validation for create/update operations

.gitignore

@@ -19,3 +19,5 @@
 # Local test data
 /testdata
 .env
+
+/.testbin/

Makefile

@@ -2,11 +2,14 @@ PROTOC = $(shell which protoc)
 PROTOC_GEN_GO = $(shell which protoc-gen-go)
 BUILDTIME = $(shell date "+%s")
 DATE = $(shell date "+%Y-%m-%d")
+K8S_VERSION := 1.27.1
 LAST_COMMIT = $(shell git rev-parse --short HEAD)
 VERSION ?= $(DATE)-$(LAST_COMMIT)
 LDFLAGS := -X github.com/nais/deploy/pkg/version.Revision=$(LAST_COMMIT) -X github.com/nais/deploy/pkg/version.Date=$(DATE) -X github.com/nais/deploy/pkg/version.BuildUnixTime=$(BUILDTIME)
-arch := amd64
+arch := $(shell uname -m | sed s/aarch64/arm64/ | sed s/x86_64/amd64/)
 os := $(shell uname -s | tr '[:upper:]' '[:lower:]')
+testbin_dir := ./.testbin/
+tools_archive := kubebuilder-tools-${K8S_VERSION}-$(os)-$(arch).tar.gz
 
 .PHONY: all proto hookd deployd token-generator deploy alpine test docker upload deploy-alpine hookd-alpine deployd-alpine
 
@@ -60,15 +63,19 @@ alpine:
 	go build -a -installsuffix cgo -o bin/deployd -ldflags "-s $(LDFLAGS)" cmd/deployd/main.go
 	go build -a -installsuffix cgo -o bin/deploy -ldflags "-s $(LDFLAGS)" cmd/deploy/main.go
 
-test:
+test: kubebuilder
 	go test ./... -count=1
 
 migration:
 	go generate ./...
 
-kubebuilder:
-	curl -L https://github.com/kubernetes-sigs/kubebuilder/releases/download/v2.3.1/kubebuilder_2.3.1_${os}_${arch}.tar.gz | tar -xz -C /tmp/
-	mv /tmp/kubebuilder_2.3.1_${os}_${arch} /usr/local/kubebuilder
+kubebuilder: $(testbin_dir)/$(tools_archive)
+	tar -xzf $(testbin_dir)/$(tools_archive) --strip-components=2 -C $(testbin_dir)
+	chmod -R +x $(testbin_dir)
+
+$(testbin_dir)/$(tools_archive):
+	mkdir -p $(testbin_dir)
+	curl -L -O --output-dir $(testbin_dir) "https://storage.googleapis.com/kubebuilder-tools/$(tools_archive)"
 
 check:
 	go run honnef.co/go/tools/cmd/staticcheck ./...

pkg/deployd/deployd/deployd.go

@@ -60,15 +60,7 @@ func Run(op *operation.Operation, client kubeclient.Interface) {
 			"gvk":       identifier.GroupVersionKind,
 		})
 
-		deployclient, err := client.WarningHandler(op.Request.GetID(), logger, resource)
-		if err != nil {
-			err = fmt.Errorf("%s: creating deploy client: %s", identifier.String(), err)
-			logger.Error(err)
-			errors <- err
-			break
-		}
-
-		resourceInterface, err := deployclient.ResourceInterface(&resource)
+		resourceInterface, err := client.ResourceInterface(&resource)
 		if err == nil {
 			_, err = strategy.NewDeployStrategy(resourceInterface).Deploy(op.Context, resource)
 		}

pkg/deployd/deployd/deployd_test.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"path/filepath"
+	go_runtime "runtime"
 	"sync"
 	"testing"
 	"time"
@@ -154,41 +156,21 @@ var tests = []testSpec{
 		fixture: "testdata/application-unknownfields-create.json",
 		timeout: 2 * time.Second,
 		endStatus: &pb.DeploymentStatus{
-			State:   pb.DeploymentState_success,
-			Message: "Deployment completed successfully.",
-		},
-		deployedResources: []client.Object{
-			&nais_io_v1alpha1.Application{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "myapplication-unknown-fields",
-					Namespace: "aura",
-				},
-			},
-		},
-		processing: func(ctx context.Context, rig *testRig, test testSpec) error {
-			return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.RolloutComplete, "completed", "myapplication-unknown-fields"))
+			State:   pb.DeploymentState_failure,
+			Message: "nais.io/v1alpha1, Kind=Application, Namespace=aura, Name=myapplication-unknown-fields: creating resource: strict decoding error:\n| ⚠️ unknown field \"spec.nestedField\"\n| ⚠️ unknown field \"spec.unknownField\"\n| The fields might be misspelled, incorrectly indented, or unsupported.\n| Please verify your resource against the reference documentation at https://doc.nais.io/nais-application/application/ (total of 1 errors)",
 		},
+		deployedResources: nil,
 	},
 
 	// Update existing Application with unknown fields
 	{
 		fixture: "testdata/application-unknownfields-update.json",
 		timeout: 2 * time.Second,
 		endStatus: &pb.DeploymentStatus{
-			State:   pb.DeploymentState_success,
-			Message: "Deployment completed successfully.",
-		},
-		deployedResources: []client.Object{
-			&nais_io_v1alpha1.Application{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "myapplication-unknown-fields-update",
-					Namespace: "aura",
-				},
-			},
-		},
-		processing: func(ctx context.Context, rig *testRig, test testSpec) error {
-			return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.RolloutComplete, "completed", "myapplication-unknown-fields-update"))
+			State:   pb.DeploymentState_failure,
+			Message: "nais.io/v1alpha1, Kind=Application, Namespace=aura, Name=myapplication: updating resource: strict decoding error:\n| ⚠️ unknown field \"spec.nestedField\"\n| ⚠️ unknown field \"spec.unknownField\"\n| The fields might be misspelled, incorrectly indented, or unsupported.\n| Please verify your resource against the reference documentation at https://doc.nais.io/nais-application/application/ (total of 1 errors)",
 		},
+		deployedResources: nil,
 	},
 }
 
@@ -200,11 +182,21 @@ type testRig struct {
 	scheme     *runtime.Scheme
 }
 
+func testBinDirectory() string {
+	_, filename, _, _ := go_runtime.Caller(0)
+	return filepath.Clean(filepath.Join(filepath.Dir(filename), "../../../.testbin/"))
+}
+
 func newTestRig() (*testRig, error) {
 	var err error
 
 	rig := &testRig{}
 
+	err = os.Setenv("KUBEBUILDER_ASSETS", testBinDirectory())
+	if err != nil {
+		return nil, fmt.Errorf("failed to set environment variable: %w", err)
+	}
+
 	rig.scheme, err = scheme.All()
 	if err != nil {
 		return nil, fmt.Errorf("initialize Kubernetes schemes: %s", err)

pkg/deployd/deployd/testdata/application-unknownfields-update.json

@@ -3,7 +3,7 @@
     "kind": "Application",
     "apiVersion": "nais.io/v1alpha1",
     "metadata": {
-      "name": "myapplication-unknown-fields-update",
+      "name": "myapplication",
       "namespace": "aura"
     },
     "spec": {

pkg/deployd/kubeclient/kubeclient.go

@@ -3,7 +3,6 @@ package kubeclient
 import (
 	"fmt"
 
-	log "github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
@@ -25,9 +24,6 @@ type Interface interface {
 
 	// Return a new client of the same type, but using the team's credentials
 	Impersonate(team string) (Interface, error)
-
-	// Return a new client of the same type, but with a WarningHandler configured.
-	WarningHandler(correlationID string, logger *log.Entry, resource unstructured.Unstructured) (Interface, error)
 }
 
 type client struct {
@@ -50,18 +46,6 @@ func (c *client) Impersonate(team string) (Interface, error) {
 	return New(config)
 }
 
-func (c *client) WarningHandler(correlationID string, logger *log.Entry, resource unstructured.Unstructured) (Interface, error) {
-	config := *c.config
-	config.WarningHandler = &warningHandler{
-		client:        c.static,
-		correlationID: correlationID,
-		logger:        logger,
-		resource:      resource,
-	}
-
-	return New(&config)
-}
-
 // Given a unstructured Kubernetes resource, return a dynamic client that knows how to apply it to the cluster.
 func (c *client) ResourceInterface(resource *unstructured.Unstructured) (dynamic.ResourceInterface, error) {
 	gvr, err := c.gvr(resource)

pkg/deployd/metrics/metrics.go

@@ -5,17 +5,11 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
-
-	"github.com/nais/deploy/pkg/k8sutils"
 )
 
 const (
 	namespace = "deployment"
 	subsystem = "deployd"
-
-	labelName      = "name"
-	labelNamespace = "namespace"
-	labelGvk       = "gvk"
 )
 
 func counter(name, help string) prometheus.Counter {
@@ -32,35 +26,12 @@ var (
 	DeployFailed        = counter("deploy_failed", "number of failed deployments")
 	DeployIgnored       = counter("deploy_ignored", "number of ignored/discarded deployments")
 	KubernetesResources = counter("kubernetes_resources", "number of Kubernetes resources successfully committed to cluster")
-
-	deployFieldValidationWarnings = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Name:      "deploy_field_validation_warnings",
-		Help:      "number of deployments with field validation warnings",
-		Namespace: namespace,
-		Subsystem: subsystem,
-	},
-		[]string{
-			labelName,
-			labelNamespace,
-			labelGvk,
-		},
-	)
 )
 
-func DeployFieldValidationWarning(identifier k8sutils.Identifier) {
-	labels := prometheus.Labels{
-		labelName:      identifier.Name,
-		labelNamespace: identifier.Namespace,
-		labelGvk:       identifier.GroupVersionKind.String(),
-	}
-	deployFieldValidationWarnings.With(labels).Inc()
-}
-
 func init() {
 	prometheus.MustRegister(DeploySuccessful)
 	prometheus.MustRegister(DeployFailed)
 	prometheus.MustRegister(DeployIgnored)
-	prometheus.MustRegister(deployFieldValidationWarnings)
 	prometheus.MustRegister(KubernetesResources)
 }