diff --git a/.gitignore b/.gitignore index 58aef2f8..0cd25bc5 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,5 @@ # Local test data /testdata .env + +/.testbin/ diff --git a/Makefile b/Makefile index b768d176..9f6bca1a 100644 --- a/Makefile +++ b/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 ./... diff --git a/pkg/deployd/deployd/deployd.go b/pkg/deployd/deployd/deployd.go index 1b218115..41616310 100644 --- a/pkg/deployd/deployd/deployd.go +++ b/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) } diff --git a/pkg/deployd/deployd/deployd_test.go b/pkg/deployd/deployd/deployd_test.go index 81449369..2fc7843e 100644 --- a/pkg/deployd/deployd/deployd_test.go +++ b/pkg/deployd/deployd/deployd_test.go @@ -4,6 +4,8 @@ import ( "context" "fmt" "os" + "path/filepath" + go_runtime "runtime" "sync" "testing" "time" @@ -154,20 +156,10 @@ 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 @@ -175,20 +167,10 @@ var tests = []testSpec{ 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) diff --git a/pkg/deployd/deployd/testdata/application-unknownfields-update.json b/pkg/deployd/deployd/testdata/application-unknownfields-update.json index 53afee9b..fd4eca68 100644 --- a/pkg/deployd/deployd/testdata/application-unknownfields-update.json +++ b/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": { diff --git a/pkg/deployd/kubeclient/kubeclient.go b/pkg/deployd/kubeclient/kubeclient.go index f89e7320..7b9e1e1d 100644 --- a/pkg/deployd/kubeclient/kubeclient.go +++ b/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) diff --git a/pkg/deployd/kubeclient/warninghandler.go b/pkg/deployd/kubeclient/warninghandler.go deleted file mode 100644 index 41c835fb..00000000 --- a/pkg/deployd/kubeclient/warninghandler.go +++ /dev/null @@ -1,145 +0,0 @@ -package kubeclient - -import ( - "context" - "fmt" - "hash/crc32" - "strings" - "time" - - nais_io_v1 "github.com/nais/liberator/pkg/apis/nais.io/v1" - log "github.com/sirupsen/logrus" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/util/validation" - "k8s.io/client-go/kubernetes" - - "github.com/nais/deploy/pkg/deployd/metrics" - "github.com/nais/deploy/pkg/k8sutils" -) - -type warningHandler struct { - client kubernetes.Interface - correlationID string - logger *log.Entry - resource unstructured.Unstructured -} - -func (w *warningHandler) HandleWarningHeader(_ int, _ string, message string) { - // there doesn't appear to be any structured way of categorizing warnings, so this function will catch _all_ warnings. - // this is also invoked for each individual warning; there can be multiple warnings when performing a single CRUD operation - w.logger.Warnf("apiserver: %s", message) - - // only count relevant warnings - if !strings.Contains(message, "unknown field") { - return - } - - metrics.DeployFieldValidationWarning(k8sutils.ResourceIdentifier(w.resource)) - - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - eventsClient := w.client.CoreV1().Events(w.resource.GetNamespace()) - - name, err := w.generateEventName(message) - if err != nil { - w.logger.Warnf("generating event name: %+v", err) - return - } - - event, err := eventsClient.Get(ctx, name, metav1.GetOptions{}) - if errors.IsNotFound(err) { - _, err := eventsClient.Create(ctx, w.makeNewEvent(name, message), metav1.CreateOptions{}) - if err != nil { - w.logger.Warnf("creating validation failure event: %+v", err) - } - } else if err != nil { - w.logger.Warnf("getting validation failure event: %+v", err) - } else { - _, err = eventsClient.Update(ctx, w.makeUpdateEvent(*event), metav1.UpdateOptions{}) - if err != nil { - w.logger.Warnf("updating validation failure event: %+v", err) - } - } -} - -func (w *warningHandler) makeNewEvent(name, message string) *v1.Event { - t := metav1.NewTime(time.Now()) - - docs := map[string]string{ - "nais.io/v1alpha1, Kind=Application": "https://doc.nais.io/nais-application/application/", - "nais.io/v1, Kind=Naisjob": "https://doc.nais.io/naisjob/reference/", - } - - msg := &strings.Builder{} - msg.WriteString(message + "; it might be misspelled or incorrectly indented, or unsupported by the resource API specifications.\n") - msg.WriteString("\tPlease verify your resource against the API reference") - if u, ok := docs[w.resource.GroupVersionKind().String()]; ok { - msg.WriteString(" documentation at " + u) - } - msg.WriteString(".\n") - msg.WriteString("\tThis is a validation warning only. Validation is not enforced and will not cause a deployment failure. However, the defined field will have no effect because it is invalid.") - - return &v1.Event{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: w.resource.GetNamespace(), - Annotations: map[string]string{ - nais_io_v1.DeploymentCorrelationIDAnnotation: w.correlationID, - }, - }, - InvolvedObject: v1.ObjectReference{ - Kind: w.resource.GetKind(), - Namespace: w.resource.GetNamespace(), - Name: w.resource.GetName(), - UID: w.resource.GetUID(), - APIVersion: w.resource.GetAPIVersion(), - ResourceVersion: w.resource.GetResourceVersion(), - }, - Reason: "UnknownFieldWarning", - Message: msg.String(), - FirstTimestamp: t, - LastTimestamp: t, - Count: 1, - Type: v1.EventTypeWarning, - Source: v1.EventSource{ - Component: "deployd", - }, - ReportingController: "deployd", - ReportingInstance: "deployd", - } -} - -func (w *warningHandler) makeUpdateEvent(event v1.Event) *v1.Event { - event.Count = event.Count + 1 - event.LastTimestamp = metav1.NewTime(time.Now()) - - a := event.GetAnnotations() - if a == nil { - a = make(map[string]string) - } - a[nais_io_v1.DeploymentCorrelationIDAnnotation] = w.correlationID - event.SetAnnotations(a) - - return &event -} - -// generateEventName generates an event name consisting of the resource name and a hash of the given message to avoid generating duplicate events -// the resource name is truncated if the result would be longer than the max length permitted by the DNS validation rules in RFC 1035 -func (w *warningHandler) generateEventName(message string) (string, error) { - basename := w.resource.GetName() - maxlen := validation.DNS1035LabelMaxLength - 9 - if len(basename) > maxlen { - basename = basename[:maxlen] - } - - hasher := crc32.NewIEEE() - _, err := hasher.Write([]byte(message)) - if err != nil { - return "", err - } - - return fmt.Sprintf("%s.%08x", basename, hasher.Sum32()), nil -} diff --git a/pkg/deployd/metrics/metrics.go b/pkg/deployd/metrics/metrics.go index bbffcbf4..c7e80c02 100644 --- a/pkg/deployd/metrics/metrics.go +++ b/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) } diff --git a/pkg/deployd/strategy/deploy.go b/pkg/deployd/strategy/deploy.go index a897ef0f..494dd82b 100644 --- a/pkg/deployd/strategy/deploy.go +++ b/pkg/deployd/strategy/deploy.go @@ -3,6 +3,7 @@ package strategy import ( "context" "fmt" + "strings" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -26,22 +27,79 @@ func (c createOrUpdateStrategy) Deploy(ctx context.Context, resource unstructure existing, err := c.client.Get(ctx, resource.GetName(), metav1.GetOptions{}) if errors.IsNotFound(err) { deployed, err := c.client.Create(ctx, &resource, metav1.CreateOptions{ - FieldValidation: metav1.FieldValidationWarn, + FieldValidation: metav1.FieldValidationStrict, }) if err != nil { - return nil, fmt.Errorf("creating resource: %s", err) + return nil, fmt.Errorf("creating resource: %w", transformStrictDecodingError(resource, err)) } return deployed, nil } else if err != nil { - return nil, fmt.Errorf("get existing resource: %s", err) + return nil, fmt.Errorf("get existing resource: %w", err) } resource.SetResourceVersion(existing.GetResourceVersion()) updated, err := c.client.Update(ctx, &resource, metav1.UpdateOptions{ - FieldValidation: metav1.FieldValidationWarn, + FieldValidation: metav1.FieldValidationStrict, }) if err != nil { - return nil, fmt.Errorf("updating resource: %s", err) + return nil, fmt.Errorf("updating resource: %w", transformStrictDecodingError(resource, err)) } + return updated, nil } + +func transformStrictDecodingError(resource unstructured.Unstructured, err error) error { + msg := err.Error() + + // Kubernetes doesn't expose any error types, so we have to rely on the error message for now + const strictDecodingError = "strict decoding error:" + + // we only transform strict decoding errors + if !strings.Contains(msg, strictDecodingError) { + return err + } + + // we trim the default error message as it is too verbose, e.g: + // > Application in version "v1alpha1" cannot be handled as a Application: strict decoding error: unknown field "spec.nestedField", ... + if strings.Contains(msg, strictDecodingError) { + parts := strings.SplitAfterN(msg, strictDecodingError, 2) + if len(parts) > 1 { + msg = parts[1] + } + } + + docs := map[string]string{ + "aiven.io/v1alpha1, Kind=OpenSearch": "https://doc.nais.io/persistence/open-search/#creating-a-opensearch-instance", + "aiven.io/v1alpha1, Kind=Redis": "https://doc.nais.io/persistence/redis/#creating-a-redis-instance-explicitly", + "aiven.io/v1alpha1, Kind=ServiceIntegration": "https://doc.nais.io/persistence/open-search/#serviceintegration", + "kafka.nais.io/v1, Kind=Topic": "https://doc.nais.io/persistence/kafka/topic/", + "krakend.nais.io/v1, Kind=ApiEndpoints": "https://doc.nais.io/security/apigateway/", + "monitoring.coreos.com/v1, Kind=PrometheusRule": "https://doc.nais.io/observability/alerts/#kubernetes-resources", + "nais.io/v1alpha1, Kind=Application": "https://doc.nais.io/nais-application/application/", + "nais.io/v1, Kind=Naisjob": "https://doc.nais.io/naisjob/reference/", + "unleash.nais.io/v1, Kind=ApiToken": "https://doc.nais.io/addons/unleash-next/#creating-a-new-api-token", + } + + s := &strings.Builder{} + s.WriteString(strictDecodingError) + + // multiple errors are joined as a comma separated string; split them up again + errs := strings.Split(msg, ",") + for _, e := range errs { + s.WriteString("\n| ⚠️ ") + s.WriteString(strings.TrimSpace(e)) + } + + s.WriteString("\n| The field") + if len(errs) > 1 { + s.WriteString("s") + } + s.WriteString(" might be misspelled, incorrectly indented, or unsupported.") + + s.WriteString("\n| Please verify your resource against the reference documentation") + if u, ok := docs[resource.GroupVersionKind().String()]; ok { + s.WriteString(" at " + u) + } + + return fmt.Errorf(s.String()) +}