From 82802d3a81150b1bfdf09cd0b11e72e9c5ccc2b6 Mon Sep 17 00:00:00 2001 From: Ben Ash <32777270+benashz@users.noreply.github.com> Date: Mon, 15 Jul 2024 14:20:38 -0400 Subject: [PATCH 01/10] Helm: support setting VaultAuthGlobalRef on VaultAuth (#851) --- chart/templates/_helpers.tpl | 36 +++++ .../templates/default-vault-auth-method.yaml | 4 + chart/values.yaml | 34 +++++ test/unit/default-vault-auth-method.bats | 141 ++++++++++++++++++ 4 files changed, 215 insertions(+) diff --git a/chart/templates/_helpers.tpl b/chart/templates/_helpers.tpl index c540985d0..11efbce8f 100644 --- a/chart/templates/_helpers.tpl +++ b/chart/templates/_helpers.tpl @@ -298,3 +298,39 @@ logging args {{- $ret | toYaml | nindent 8 -}} {{- end -}} {{- end -}} + +{{/* +vaultAuthGlobalRef generates the global-vault-auth-global-ref flag for the manager. +*/}} +{{- define "vso.vaulAuthGlobalRef" -}} +{{- if .Values.controller.manager.globalVaultAuthOptions.allowDefaultGlobals }} +--global-vault-auth-global-ref +{{- end -}} +{{- end -}} + +{{/* +vaultAuthGlobalRef generates the default VaultAuth spec.vaultAuthGlobalRef. +*/}} +{{- define "vso.vaultAuthGlobalRef" -}} +{{- $ret := dict -}} +{{- with .Values.defaultAuthMethod.vaultAuthGlobalRef -}} +{{ $_ := set $ret "namespace" .namespace -}} +{{ $_ = set $ret "name" .name -}} +{{ if ne .allowDefault nil -}} +{{- $_ = set $ret "allowDefault" .allowDefault -}} +{{- end -}} +{{- $strat := dict -}} +{{- if .mergeStrategy.headers -}} +{{- $_ = set $strat "headers" .mergeStrategy.headers -}} +{{- end -}} +{{- if .mergeStrategy.params -}} +{{- $_ = set $strat "params" .mergeStrategy.params -}} +{{- end -}} +{{- if $strat -}} +{{- $_ = set $ret "mergeStrategy" $strat -}} +{{- end -}} +{{- end -}} +{{- if $ret -}} +{{- $ret | toYaml | nindent 4 -}} +{{- end -}} +{{- end -}} diff --git a/chart/templates/default-vault-auth-method.yaml b/chart/templates/default-vault-auth-method.yaml index 8cd264fdd..2c2232976 100644 --- a/chart/templates/default-vault-auth-method.yaml +++ b/chart/templates/default-vault-auth-method.yaml @@ -24,4 +24,8 @@ spec: mount: {{ .Values.defaultAuthMethod.mount }} {{- $kubeServiceAccount := .Values.defaultAuthMethod.kubernetes.serviceAccount }} {{- include "vso.vaultAuthMethod" (list .Values.defaultAuthMethod $kubeServiceAccount . ) }} + {{- if .Values.defaultAuthMethod.vaultAuthGlobalRef.enabled }} + vaultAuthGlobalRef: + {{- include "vso.vaultAuthGlobalRef" . }} + {{- end }} {{- end }} diff --git a/chart/values.yaml b/chart/values.yaml index 75788e79d..7c002c481 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -746,6 +746,40 @@ defaultAuthMethod: # @type: map headers: {} + # VaultAuthGlobalRef + vaultAuthGlobalRef: + # toggles the inclusion of the VaultAuthGlobal configuration in the + # default VaultAuth CR. + # @type: boolean + enabled: false + # Name of the VaultAuthGlobal CR to reference. + # @type: string + name: "" + + # Namespace of the VaultAuthGlobal CR to reference. + # @type: string + namespace: "" + + # allow default globals + # @type: boolean + allowDefault: + + mergeStrategy: + # merge strategy for headers + # @type: string + # Valid values are: "replace", "merge", "none" + # Default: "replace" + # @type: string + headers: none + + # merge strategy for params + # @type: string + # Valid values are: "replace", "merge", "none" + # Default: "replace" + # @type: string + params: none + + # Configures a Prometheus ServiceMonitor telemetry: serviceMonitor: diff --git a/test/unit/default-vault-auth-method.bats b/test/unit/default-vault-auth-method.bats index 1a9e07ab8..4ac4745eb 100755 --- a/test/unit/default-vault-auth-method.bats +++ b/test/unit/default-vault-auth-method.bats @@ -1,5 +1,10 @@ #!/usr/bin/env bats +# +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: BUSL-1.1 +# + load _helpers #-------------------------------------------------------------------- @@ -398,3 +403,139 @@ load _helpers actual=$(echo "$object" | yq '.spec.gcp.projectID' | tee /dev/stderr) [ "${actual}" = "my-project" ] } + +@test "defaultAuthMethod/CR: with vaultAuthGlobalRef/default" { + cd "$(chart_dir)" + local actual + actual=$(helm template \ + --debug \ + -s templates/default-vault-auth-method.yaml \ + --set 'defaultAuthMethod.enabled=true' \ + . | tee /dev/stderr | + yq '.spec' | tee /dev/stderr) + + [ "$(echo "$actual" | yq '. | has("vaultAuthGlobalRef")')" = "false" ] +} + +@test "defaultAuthMethod/CR: with vaultAuthGlobalRef/enabled" { + cd "$(chart_dir)" + local actual + actual=$(helm template \ + --debug \ + -s templates/default-vault-auth-method.yaml \ + --set 'defaultAuthMethod.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.name=foo' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.namespace=baz' \ + . | tee /dev/stderr | + yq '.spec' | tee /dev/stderr) + + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef | has("allowDefault")')" = "false" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.name')" = "foo" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.namespace')" = "baz" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.mergeStrategy.params')" = "none" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.mergeStrategy.headers')" = "none" ] +} + +@test "defaultAuthMethod/CR: with vaultAuthGlobalRef/defaults/empty-params" { + cd "$(chart_dir)" + local actual + actual=$(helm template \ + --debug \ + -s templates/default-vault-auth-method.yaml \ + --set 'defaultAuthMethod.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.name=foo' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.namespace=baz' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.mergeStrategy.params=' \ + . | tee /dev/stderr | + yq '.spec' | tee /dev/stderr) + + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef | has("allowDefault")')" = "false" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.name')" = "foo" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.namespace')" = "baz" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.mergeStrategy | has("params")')" = "false" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.mergeStrategy.headers')" = "none" ] +} + +@test "defaultAuthMethod/CR: with vaultAuthGlobalRef/mergeStrategy/empty-headers" { + cd "$(chart_dir)" + local actual + actual=$(helm template \ + --debug \ + -s templates/default-vault-auth-method.yaml \ + --set 'defaultAuthMethod.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.name=foo' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.namespace=baz' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.mergeStrategy.headers=' \ + . | tee /dev/stderr | + yq '.spec' | tee /dev/stderr) + + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef | has("allowDefault")')" = "false" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.name')" = "foo" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.namespace')" = "baz" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.mergeStrategy.params')" = "none" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.mergeStrategy | has("headers")')" = "false" ] +} + +@test "defaultAuthMethod/CR: with vaultAuthGlobalRef/allowDefault=true" { + cd "$(chart_dir)" + local actual + actual=$(helm template \ + --debug \ + -s templates/default-vault-auth-method.yaml \ + --set 'defaultAuthMethod.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.name=foo' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.namespace=baz' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.allowDefault=true' \ + . | tee /dev/stderr | + yq '.spec' | tee /dev/stderr) + + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.allowDefault')" = "true" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.name')" = "foo" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.namespace')" = "baz" ] +} + +@test "defaultAuthMethod/CR: with vaultAuthGlobalRef/allowDefault=false" { + cd "$(chart_dir)" + local actual + actual=$(helm template \ + --debug \ + -s templates/default-vault-auth-method.yaml \ + --set 'defaultAuthMethod.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.name=foo' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.namespace=baz' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.allowDefault=false' \ + . | tee /dev/stderr | + yq '.spec' | tee /dev/stderr) + + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.allowDefault')" = "false" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.name')" = "foo" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.namespace')" = "baz" ] +} + +@test "defaultAuthMethod/CR: with vaultAuthGlobalRef/mergeStrategy/params=union-headers=replace" { + cd "$(chart_dir)" + local actual + actual=$(helm template \ + --debug \ + -s templates/default-vault-auth-method.yaml \ + --set 'defaultAuthMethod.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.enabled=true' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.name=foo' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.namespace=baz' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.allowDefault=false' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.mergeStrategy.params=union' \ + --set 'defaultAuthMethod.vaultAuthGlobalRef.mergeStrategy.headers=replace' \ + . | tee /dev/stderr | + yq '.spec' | tee /dev/stderr) + + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.allowDefault')" = "false" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.name')" = "foo" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.namespace')" = "baz" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.mergeStrategy.params')" = "union" ] + [ "$(echo "$actual" | yq '.vaultAuthGlobalRef.mergeStrategy.headers')" = "replace" ] +} From eec0e9f759039dfca148e21be582ae7ff8e43968 Mon Sep 17 00:00:00 2001 From: Ben Ash <32777270+benashz@users.noreply.github.com> Date: Mon, 15 Jul 2024 14:21:19 -0400 Subject: [PATCH 02/10] Rejig VA status conditions (#850) Updates VaultAuthGlobal conditions to more in line with the other core K8s resources. --- controllers/common.go | 46 ++++++++ controllers/common_test.go | 161 ++++++++++++++++++++++++++++ controllers/vaultauth_controller.go | 17 ++- 3 files changed, 215 insertions(+), 9 deletions(-) diff --git a/controllers/common.go b/controllers/common.go index d4f99f161..07228fdf8 100644 --- a/controllers/common.go +++ b/controllers/common.go @@ -10,6 +10,7 @@ import ( "time" "github.com/go-logr/logr" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" @@ -270,3 +271,48 @@ func waitForStoppedCh(ctx context.Context, stoppedCh chan struct{}) error { return ctx.Err() } } + +// updateConditions updates the current conditions with updates, returning a new +// set of metav1.Condition(s). It will update the LastTransitionTime if the condition has +// changed. It will also append new conditions to the existing conditions. All +// updates are deduplicated based on their type and reason. +func updateConditions(current []metav1.Condition, updates ...metav1.Condition) []metav1.Condition { + seen := make(map[string]bool) + var ret []metav1.Condition + for _, newCond := range updates { + // we key conditions on their type and reason + // e.g: type=VaultAuthGlobal reason=Available, ... + key := fmt.Sprintf("%s/%s", newCond.Type, newCond.Reason) + if seen[key] { + // drop duplicate conditions + continue + } + + seen[key] = true + var updated bool + for _, cond := range current { + if cond.Type == newCond.Type && cond.Reason == newCond.Reason { + if cond.Status != newCond.Status { + newCond.LastTransitionTime = metav1.NewTime(nowFunc()) + } + if newCond.LastTransitionTime.IsZero() { + if cond.LastTransitionTime.IsZero() { + newCond.LastTransitionTime = metav1.NewTime(nowFunc()) + } else { + newCond.LastTransitionTime = cond.LastTransitionTime + } + } + ret = append(ret, newCond) + updated = true + break + } + } + if !updated { + if newCond.LastTransitionTime.IsZero() { + newCond.LastTransitionTime = metav1.NewTime(nowFunc()) + } + ret = append(ret, newCond) + } + } + return ret +} diff --git a/controllers/common_test.go b/controllers/common_test.go index e174a3c5a..b3b7b6563 100644 --- a/controllers/common_test.go +++ b/controllers/common_test.go @@ -398,3 +398,164 @@ func Test_waitForStoppedCh(t *testing.T) { err = waitForStoppedCh(ctx2, stoppedCh) assert.NoError(t, err) } + +func TestVaultAuthReconciler_updateConditions(t *testing.T) { + t0 := nowFunc().Truncate(time.Second) + tests := []struct { + name string + current []metav1.Condition + updates []metav1.Condition + expectLen int + }{ + { + name: "no-conditions", + }, + { + name: "initial-conditions", + updates: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionTrue, + }, + }, + expectLen: 1, + }, + { + name: "unchanged-conditions", + current: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.NewTime(t0), + }, + }, + updates: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionTrue, + }, + }, + expectLen: 1, + }, + { + name: "unchanged-conditions-no-last-transition-time", + current: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionTrue, + }, + }, + updates: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionTrue, + }, + }, + expectLen: 1, + }, + { + name: "unchanged-conditions-with-duplicate-updates", + current: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.NewTime(t0), + }, + }, + updates: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.NewTime(t0), + }, + { + Type: "Available", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.NewTime(t0), + }, + }, + expectLen: 1, + }, + { + name: "condition-status-transition", + current: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionFalse, + LastTransitionTime: metav1.NewTime(nowFunc().Add(-time.Minute)), + }, + { + Type: "Progressing", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.NewTime(nowFunc().Add(-time.Minute)), + }, + }, + updates: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionTrue, + }, + { + Type: "Progressing", + Status: metav1.ConditionFalse, + }, + }, + expectLen: 2, + }, + { + name: "condition-status-transition-appending-new-condition", + current: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionFalse, + LastTransitionTime: metav1.NewTime(nowFunc().Add(-time.Minute)), + }, + }, + updates: []metav1.Condition{ + { + Type: "Available", + Status: metav1.ConditionTrue, + }, + { + Type: "Progressing", + Status: metav1.ConditionFalse, + }, + }, + expectLen: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + origUpdates := make([]metav1.Condition, len(tt.updates)) + seen := make(map[string]bool) + for idx, cond := range tt.updates { + key := fmt.Sprintf("%s/%s", cond.Type, cond.Reason) + if seen[key] { + continue + } + seen[key] = true + origUpdates[idx] = cond + } + + got := updateConditions(tt.current, tt.updates...) + assert.Equalf(t, tt.expectLen, len(got), + "expected %d conditions, got %d", tt.expectLen, len(got)) + for _, orig := range origUpdates { + for _, cond := range got { + if cond.Reason == orig.Reason && orig.Status == cond.Status { + if orig.LastTransitionTime.IsZero() { + assert.False(t, cond.LastTransitionTime.IsZero()) + } else { + assert.Equal(t, orig, cond) + } + } else if cond.Reason == orig.Reason && orig.Status != cond.Status { + assert.Greater(t, cond.LastTransitionTime.Time.Unix(), orig.LastTransitionTime.Time.Unix()) + } else { + // not updated + assert.False(t, orig.LastTransitionTime.IsZero()) + } + } + } + }) + } +} diff --git a/controllers/vaultauth_controller.go b/controllers/vaultauth_controller.go index d424c0cf6..11cdd5ffd 100644 --- a/controllers/vaultauth_controller.go +++ b/controllers/vaultauth_controller.go @@ -82,20 +82,20 @@ func (r *VaultAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( var conditions []metav1.Condition if o.Spec.VaultAuthGlobalRef != nil { condition := metav1.Condition{ - Type: "VaultAuthGlobalRef", - Status: "True", + Type: "Available", + Status: metav1.ConditionTrue, ObservedGeneration: o.Generation, - LastTransitionTime: metav1.NewTime(nowFunc()), Reason: "VaultAuthGlobalRef", } mObj, gObj, err := common.MergeInVaultAuthGlobal(ctx, r.Client, o, r.GlobalVaultAuthOptions) if err != nil { condition.Message = err.Error() - condition.Status = "False" + condition.Status = metav1.ConditionFalse } else { o = mObj - condition.Message = fmt.Sprintf("%s:%s:%d", + condition.Message = fmt.Sprintf( + "VaultAuthGlobal successfully merged, key=%s, uid=%s, generation=%d", client.ObjectKeyFromObject(gObj), gObj.UID, gObj.Generation) r.referenceCache.Set( VaultAuthGlobal, req.NamespacedName, @@ -106,8 +106,6 @@ func (r *VaultAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( r.referenceCache.Remove(VaultAuthGlobal, req.NamespacedName) } - o.Status.Conditions = conditions - // ensure that the vaultConnectionRef is set for any VaultAuth resource in the operator namespace. if o.Namespace == common.OperatorNamespace && o.Spec.VaultConnectionRef == "" { err = fmt.Errorf("vaultConnectionRef must be set on resources in the %q namespace", common.OperatorNamespace) @@ -178,7 +176,7 @@ func (r *VaultAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( o.Status.Error = "" } - if err := r.updateStatus(ctx, o); err != nil { + if err := r.updateStatus(ctx, o, conditions...); err != nil { return ctrl.Result{}, err } @@ -195,9 +193,10 @@ func (r *VaultAuthReconciler) recordEvent(a *secretsv1beta1.VaultAuth, reason, m r.Recorder.Eventf(a, eventType, reason, msg, i...) } -func (r *VaultAuthReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.VaultAuth) error { +func (r *VaultAuthReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.VaultAuth, conditions ...metav1.Condition) error { logger := log.FromContext(ctx) metrics.SetResourceStatus("vaultauth", o, o.Status.Valid) + o.Status.Conditions = updateConditions(o.Status.Conditions, conditions...) if err := r.Status().Update(ctx, o); err != nil { logger.Error(err, "Failed to update the resource's status") return err From 67d4ec025b028402050c020d4397db98e5fec673 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Jul 2024 14:52:54 -0400 Subject: [PATCH 03/10] Bump github.com/gruntwork-io/terratest from 0.46.16 to 0.47.0 (#852) Bumps [github.com/gruntwork-io/terratest](https://github.com/gruntwork-io/terratest) from 0.46.16 to 0.47.0. - [Release notes](https://github.com/gruntwork-io/terratest/releases) - [Commits](https://github.com/gruntwork-io/terratest/compare/v0.46.16...v0.47.0) --- updated-dependencies: - dependency-name: github.com/gruntwork-io/terratest dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 9ea847af4..ddc2cc139 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/go-openapi/runtime v0.28.0 github.com/go-openapi/strfmt v0.23.0 github.com/google/uuid v1.6.0 - github.com/gruntwork-io/terratest v0.46.16 + github.com/gruntwork-io/terratest v0.47.0 github.com/hashicorp/go-hclog v1.6.3 github.com/hashicorp/go-rootcerts v1.0.2 github.com/hashicorp/go-secure-stdlib/awsutil v0.3.0 diff --git a/go.sum b/go.sum index 3d5d0e0a7..d9b747e48 100644 --- a/go.sum +++ b/go.sum @@ -442,8 +442,8 @@ github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/ad github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/gruntwork-io/go-commons v0.8.0 h1:k/yypwrPqSeYHevLlEDmvmgQzcyTwrlZGRaxEM6G0ro= github.com/gruntwork-io/go-commons v0.8.0/go.mod h1:gtp0yTtIBExIZp7vyIV9I0XQkVwiQZze678hvDXof78= -github.com/gruntwork-io/terratest v0.46.16 h1:l+HHuU7lNLwoAl2sP8zkYJy0uoE2Mwha2nw+rim+OhQ= -github.com/gruntwork-io/terratest v0.46.16/go.mod h1:oywHw1cFKXSYvKPm27U7quZVzDUlA22H2xUrKCe26xM= +github.com/gruntwork-io/terratest v0.47.0 h1:xIy1pT7NbGVlMLDZEHl3+3iSnvffh8tN2pL6idn448c= +github.com/gruntwork-io/terratest v0.47.0/go.mod h1:oywHw1cFKXSYvKPm27U7quZVzDUlA22H2xUrKCe26xM= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= From a916d649b3ec31e1e76dcceaaee19425d69e385b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Jul 2024 14:53:06 -0400 Subject: [PATCH 04/10] Bump cloud.google.com/go/compute/metadata from 0.4.0 to 0.5.0 (#853) Bumps [cloud.google.com/go/compute/metadata](https://github.com/googleapis/google-cloud-go) from 0.4.0 to 0.5.0. - [Release notes](https://github.com/googleapis/google-cloud-go/releases) - [Changelog](https://github.com/googleapis/google-cloud-go/blob/main/CHANGES.md) - [Commits](https://github.com/googleapis/google-cloud-go/compare/v0.4.0...v0.5.0) --- updated-dependencies: - dependency-name: cloud.google.com/go/compute/metadata dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index ddc2cc139..481ddcac4 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.22.0 toolchain go1.22.2 require ( - cloud.google.com/go/compute/metadata v0.4.0 + cloud.google.com/go/compute/metadata v0.5.0 github.com/Masterminds/sprig/v3 v3.2.3 github.com/argoproj/argo-rollouts v1.6.6 github.com/cenkalti/backoff/v4 v4.3.0 diff --git a/go.sum b/go.sum index d9b747e48..c99f88c7d 100644 --- a/go.sum +++ b/go.sum @@ -72,8 +72,8 @@ cloud.google.com/go/compute v1.6.0/go.mod h1:T29tfhtVbq1wvAPo0E3+7vhgmkOYeXjhFvz cloud.google.com/go/compute v1.6.1/go.mod h1:g85FgpzFvNULZ+S8AYq87axRKuf2Kh7deLqV/jJ3thU= cloud.google.com/go/compute v1.7.0/go.mod h1:435lt8av5oL9P3fv1OEzSbSUe+ybHXGMPQHHZWZxy9U= cloud.google.com/go/compute v1.10.0/go.mod h1:ER5CLbMxl90o2jtNbGSbtfOpQKR0t15FOtRsugnLrlU= -cloud.google.com/go/compute/metadata v0.4.0 h1:vHzJCWaM4g8XIcm8kopr3XmDA4Gy/lblD3EhhSux05c= -cloud.google.com/go/compute/metadata v0.4.0/go.mod h1:SIQh1Kkb4ZJ8zJ874fqVkslA29PRXuleyj6vOzlbK7M= +cloud.google.com/go/compute/metadata v0.5.0 h1:Zr0eK8JbFv6+Wi4ilXAR8FJ3wyNdpxHKJNPos6LTZOY= +cloud.google.com/go/compute/metadata v0.5.0/go.mod h1:aHnloV2TPI38yx4s9+wAZhHykWvVCfu7hQbF+9CWoiY= cloud.google.com/go/containeranalysis v0.5.1/go.mod h1:1D92jd8gRR/c0fGMlymRgxWD3Qw9C1ff6/T7mLgVL8I= cloud.google.com/go/containeranalysis v0.6.0/go.mod h1:HEJoiEIu+lEXM+k7+qLCci0h33lX3ZqoYFdmPcoO7s4= cloud.google.com/go/datacatalog v1.3.0/go.mod h1:g9svFY6tuR+j+hrTw3J2dNcmI0dzmSiyOzm8kpLq0a0= From f2bac619ea40d5be829e03a44ac9ca7a1f2b285c Mon Sep 17 00:00:00 2001 From: Ben Ash <32777270+benashz@users.noreply.github.com> Date: Tue, 16 Jul 2024 15:04:00 -0400 Subject: [PATCH 05/10] VaultAuth: set valid status on VaultAuthGlobal deref error (#854) VaultAuth: set valid status on VAG deref error Other fixes: - For all applicable types, make .Status.Types a boolean pointer so that it can be stored as false --- api/v1beta1/hcpauth_types.go | 8 ++--- api/v1beta1/secrettransformation_types.go | 8 ++--- api/v1beta1/vaultauth_types.go | 2 +- api/v1beta1/vaultconnection_types.go | 8 ++--- api/v1beta1/vaultpkisecret_types.go | 2 +- api/v1beta1/zz_generated.deepcopy.go | 33 ++++++++++++++++--- controllers/hcpauth_controller.go | 17 +++++----- .../secrettransformation_controller.go | 15 +++++---- controllers/vaultauth_controller.go | 31 ++++++++++++----- controllers/vaultconnection_controller.go | 20 ++++++----- controllers/vaultpkisecret_controller.go | 13 ++++---- internal/helpers/template.go | 3 +- internal/helpers/template_test.go | 5 +-- 13 files changed, 105 insertions(+), 60 deletions(-) diff --git a/api/v1beta1/hcpauth_types.go b/api/v1beta1/hcpauth_types.go index eac66b146..b2d671fe0 100644 --- a/api/v1beta1/hcpauth_types.go +++ b/api/v1beta1/hcpauth_types.go @@ -49,12 +49,12 @@ type HCPAuthServicePrincipal struct { // HCPAuthStatus defines the observed state of HCPAuth type HCPAuthStatus struct { // Valid auth mechanism. - Valid bool `json:"valid"` + Valid *bool `json:"valid"` Error string `json:"error"` } -//+kubebuilder:object:root=true -//+kubebuilder:subresource:status +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status // HCPAuth is the Schema for the hcpauths API type HCPAuth struct { @@ -65,7 +65,7 @@ type HCPAuth struct { Status HCPAuthStatus `json:"status,omitempty"` } -//+kubebuilder:object:root=true +// +kubebuilder:object:root=true // HCPAuthList contains a list of HCPAuth type HCPAuthList struct { diff --git a/api/v1beta1/secrettransformation_types.go b/api/v1beta1/secrettransformation_types.go index c92e060c7..01a68f28e 100644 --- a/api/v1beta1/secrettransformation_types.go +++ b/api/v1beta1/secrettransformation_types.go @@ -9,12 +9,12 @@ import ( // SecretTransformationStatus defines the observed state of SecretTransformation type SecretTransformationStatus struct { - Valid bool `json:"valid"` + Valid *bool `json:"valid"` Error string `json:"error"` } -//+kubebuilder:object:root=true -//+kubebuilder:subresource:status +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status // SecretTransformation is the Schema for the secrettransformations API type SecretTransformation struct { @@ -55,7 +55,7 @@ type SourceTemplate struct { Text string `json:"text"` } -//+kubebuilder:object:root=true +// +kubebuilder:object:root=true // SecretTransformationList contains a list of SecretTransformation type SecretTransformationList struct { diff --git a/api/v1beta1/vaultauth_types.go b/api/v1beta1/vaultauth_types.go index 10025b72e..0c8c96ade 100644 --- a/api/v1beta1/vaultauth_types.go +++ b/api/v1beta1/vaultauth_types.go @@ -428,7 +428,7 @@ type VaultAuthSpec struct { // VaultAuthStatus defines the observed state of VaultAuth type VaultAuthStatus struct { // Valid auth mechanism. - Valid bool `json:"valid,omitempty"` + Valid *bool `json:"valid,omitempty"` Error string `json:"error,omitempty"` Conditions []metav1.Condition `json:"conditions,omitempty"` SpecHash string `json:"specHash,omitempty"` diff --git a/api/v1beta1/vaultconnection_types.go b/api/v1beta1/vaultconnection_types.go index 7a2802996..c7147b42d 100644 --- a/api/v1beta1/vaultconnection_types.go +++ b/api/v1beta1/vaultconnection_types.go @@ -28,11 +28,11 @@ type VaultConnectionSpec struct { // VaultConnectionStatus defines the observed state of VaultConnection type VaultConnectionStatus struct { // Valid auth mechanism. - Valid bool `json:"valid"` + Valid *bool `json:"valid"` } -//+kubebuilder:object:root=true -//+kubebuilder:subresource:status +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status // VaultConnection is the Schema for the vaultconnections API type VaultConnection struct { @@ -43,7 +43,7 @@ type VaultConnection struct { Status VaultConnectionStatus `json:"status,omitempty"` } -//+kubebuilder:object:root=true +// +kubebuilder:object:root=true // VaultConnectionList contains a list of VaultConnection type VaultConnectionList struct { diff --git a/api/v1beta1/vaultpkisecret_types.go b/api/v1beta1/vaultpkisecret_types.go index 9bf9c92e8..736e4868b 100644 --- a/api/v1beta1/vaultpkisecret_types.go +++ b/api/v1beta1/vaultpkisecret_types.go @@ -135,7 +135,7 @@ type VaultPKISecretStatus struct { // The SecretMac is also used to detect drift in the Destination Secret's Data. // If drift is detected the data will be synced to the Destination. SecretMAC string `json:"secretMAC,omitempty"` - Valid bool `json:"valid"` + Valid *bool `json:"valid"` Error string `json:"error"` } diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 0a90ae4a2..7e43e2683 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -48,7 +48,7 @@ func (in *HCPAuth) DeepCopyInto(out *HCPAuth) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HCPAuth. @@ -144,6 +144,11 @@ func (in *HCPAuthSpec) DeepCopy() *HCPAuthSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HCPAuthStatus) DeepCopyInto(out *HCPAuthStatus) { *out = *in + if in.Valid != nil { + in, out := &in.Valid, &out.Valid + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HCPAuthStatus. @@ -287,7 +292,7 @@ func (in *SecretTransformation) DeepCopyInto(out *SecretTransformation) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretTransformation. @@ -380,6 +385,11 @@ func (in *SecretTransformationSpec) DeepCopy() *SecretTransformationSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SecretTransformationStatus) DeepCopyInto(out *SecretTransformationStatus) { *out = *in + if in.Valid != nil { + in, out := &in.Valid, &out.Valid + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretTransformationStatus. @@ -1050,6 +1060,11 @@ func (in *VaultAuthSpec) DeepCopy() *VaultAuthSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VaultAuthStatus) DeepCopyInto(out *VaultAuthStatus) { *out = *in + if in.Valid != nil { + in, out := &in.Valid, &out.Valid + *out = new(bool) + **out = **in + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) @@ -1090,7 +1105,7 @@ func (in *VaultConnection) DeepCopyInto(out *VaultConnection) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VaultConnection. @@ -1168,6 +1183,11 @@ func (in *VaultConnectionSpec) DeepCopy() *VaultConnectionSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VaultConnectionStatus) DeepCopyInto(out *VaultConnectionStatus) { *out = *in + if in.Valid != nil { + in, out := &in.Valid, &out.Valid + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VaultConnectionStatus. @@ -1291,7 +1311,7 @@ func (in *VaultPKISecret) DeepCopyInto(out *VaultPKISecret) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VaultPKISecret. @@ -1393,6 +1413,11 @@ func (in *VaultPKISecretSpec) DeepCopy() *VaultPKISecretSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VaultPKISecretStatus) DeepCopyInto(out *VaultPKISecretStatus) { *out = *in + if in.Valid != nil { + in, out := &in.Valid, &out.Valid + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VaultPKISecretStatus. diff --git a/controllers/hcpauth_controller.go b/controllers/hcpauth_controller.go index 96db87ad7..a9f5632ed 100644 --- a/controllers/hcpauth_controller.go +++ b/controllers/hcpauth_controller.go @@ -13,6 +13,7 @@ import ( hvsclient "github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-secrets/preview/2023-06-13/client" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -27,11 +28,11 @@ type HCPAuthReconciler struct { Scheme *runtime.Scheme } -//+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=hcpauths,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=hcpauths/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=hcpauths/finalizers,verbs=update -//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch -//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch +// +kubebuilder:rbac:groups=secrets.hashicorp.com,resources=hcpauths,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=secrets.hashicorp.com,resources=hcpauths/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=secrets.hashicorp.com,resources=hcpauths/finalizers,verbs=update +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch // Reconcile validates the HCPAuth CR instance, updating the instance's status // with the result. @@ -69,10 +70,10 @@ func (r *HCPAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct logger.Error(err, "Validation failed") errs = errors.Join(err) o.Status.Error = err.Error() - o.Status.Valid = false + o.Status.Valid = pointer.Bool(false) } else { o.Status.Error = "" - o.Status.Valid = true + o.Status.Valid = pointer.Bool(true) } if err := r.updateStatus(ctx, o); err != nil { @@ -91,7 +92,7 @@ func (r *HCPAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct func (r *HCPAuthReconciler) updateStatus(ctx context.Context, a *secretsv1beta1.HCPAuth) error { logger := log.FromContext(ctx) - metrics.SetResourceStatus("hcpauth", a, a.Status.Valid) + metrics.SetResourceStatus("hcpauth", a, pointer.BoolDeref(a.Status.Valid, false)) if err := r.Status().Update(ctx, a); err != nil { logger.Error(err, "Failed to update the resource's status") return err diff --git a/controllers/secrettransformation_controller.go b/controllers/secrettransformation_controller.go index 9f7e75969..868abcd93 100644 --- a/controllers/secrettransformation_controller.go +++ b/controllers/secrettransformation_controller.go @@ -10,6 +10,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -28,10 +29,10 @@ type SecretTransformationReconciler struct { Recorder record.EventRecorder } -//+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=secrettransformations,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=secrettransformations/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=secrettransformations/finalizers,verbs=update -//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch +// +kubebuilder:rbac:groups=secrets.hashicorp.com,resources=secrettransformations,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=secrets.hashicorp.com,resources=secrettransformations/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=secrets.hashicorp.com,resources=secrettransformations/finalizers,verbs=update +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -59,11 +60,11 @@ func (r *SecretTransformationReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, nil } - o.Status.Valid = true + o.Status.Valid = pointer.Bool(true) o.Status.Error = "" errs := ValidateSecretTransformation(ctx, o) if errs != nil { - o.Status.Valid = false + o.Status.Valid = pointer.Bool(false) o.Status.Error = errs.Error() logger.Error(err, "Failed to validate configured templates") r.Recorder.Eventf(o, corev1.EventTypeWarning, consts.ReasonInvalidConfiguration, @@ -79,7 +80,7 @@ func (r *SecretTransformationReconciler) Reconcile(ctx context.Context, req ctrl func (r *SecretTransformationReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.SecretTransformation) error { logger := log.FromContext(ctx) - metrics.SetResourceStatus("secrettransformation", o, o.Status.Valid) + metrics.SetResourceStatus("secrettransformation", o, pointer.BoolDeref(o.Status.Valid, false)) if err := r.Status().Update(ctx, o); err != nil { logger.Error(err, "Failed to update the resource's status") return err diff --git a/controllers/vaultauth_controller.go b/controllers/vaultauth_controller.go index 11cdd5ffd..65fc69cee 100644 --- a/controllers/vaultauth_controller.go +++ b/controllers/vaultauth_controller.go @@ -16,6 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -76,7 +77,7 @@ func (r *VaultAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } // assume that status is always invalid - o.Status.Valid = false + o.Status.Valid = pointer.Bool(false) var errs error var conditions []metav1.Condition @@ -90,6 +91,7 @@ func (r *VaultAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( mObj, gObj, err := common.MergeInVaultAuthGlobal(ctx, r.Client, o, r.GlobalVaultAuthOptions) if err != nil { + errs = errors.Join(errs, err) condition.Message = err.Error() condition.Status = metav1.ConditionFalse } else { @@ -168,11 +170,13 @@ func (r *VaultAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( o.Status.SpecHash = specHash + var horizon time.Duration if errs != nil { - o.Status.Valid = false + o.Status.Valid = pointer.Bool(false) o.Status.Error = errs.Error() + horizon = computeHorizonWithJitter(requeueDurationOnError) } else { - o.Status.Valid = true + o.Status.Valid = pointer.Bool(true) o.Status.Error = "" } @@ -180,22 +184,31 @@ func (r *VaultAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } - r.recordEvent(o, consts.ReasonAccepted, "Successfully handled VaultAuth resource request") - return ctrl.Result{}, nil + if errs == nil { + r.recordEvent(o, consts.ReasonAccepted, "Successfully handled VaultAuth resource request") + } else { + logger.Error(errs, "Failed to handle VaultAuth resource request", "horizon", horizon) + r.recordEvent(o, consts.ReasonAccepted, + fmt.Sprintf("Failed to handle VaultAuth resource request: err=%s", errs)) + } + + return ctrl.Result{ + RequeueAfter: horizon, + }, nil } -func (r *VaultAuthReconciler) recordEvent(a *secretsv1beta1.VaultAuth, reason, msg string, i ...interface{}) { +func (r *VaultAuthReconciler) recordEvent(o *secretsv1beta1.VaultAuth, reason, msg string, i ...interface{}) { eventType := corev1.EventTypeNormal - if !a.Status.Valid { + if !pointer.BoolDeref(o.Status.Valid, false) { eventType = corev1.EventTypeWarning } - r.Recorder.Eventf(a, eventType, reason, msg, i...) + r.Recorder.Eventf(o, eventType, reason, msg, i...) } func (r *VaultAuthReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.VaultAuth, conditions ...metav1.Condition) error { logger := log.FromContext(ctx) - metrics.SetResourceStatus("vaultauth", o, o.Status.Valid) + metrics.SetResourceStatus("vaultauth", o, pointer.BoolDeref(o.Status.Valid, false)) o.Status.Conditions = updateConditions(o.Status.Conditions, conditions...) if err := r.Status().Update(ctx, o); err != nil { logger.Error(err, "Failed to update the resource's status") diff --git a/controllers/vaultconnection_controller.go b/controllers/vaultconnection_controller.go index 274ef69af..18be093c9 100644 --- a/controllers/vaultconnection_controller.go +++ b/controllers/vaultconnection_controller.go @@ -7,6 +7,8 @@ import ( "context" "errors" + "k8s.io/utils/pointer" + "github.com/hashicorp/vault-secrets-operator/internal/metrics" corev1 "k8s.io/api/core/v1" @@ -34,13 +36,13 @@ type VaultConnectionReconciler struct { ClientFactory vault.CachingClientFactory } -//+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=vaultconnections,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=vaultconnections/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=secrets.hashicorp.com,resources=vaultconnections/finalizers,verbs=update -//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch -//+kubebuilder:rbac:groups="",resources=secrets,verbs=get +// +kubebuilder:rbac:groups=secrets.hashicorp.com,resources=vaultconnections,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=secrets.hashicorp.com,resources=vaultconnections/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=secrets.hashicorp.com,resources=vaultconnections/finalizers,verbs=update +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get // needed for managing cached Clients, duplicated in vaultauth_controller.go -//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;delete;update;patch;deletecollection +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;delete;update;patch;deletecollection // Reconcile reconciles the secretsv1beta1.VaultConnection resource. // Upon a reconciliation it will verify that the configured Vault connection is valid. @@ -66,7 +68,7 @@ func (r *VaultConnectionReconciler) Reconcile(ctx context.Context, req ctrl.Requ } // assume that status is always invalid - o.Status.Valid = false + o.Status.Valid = pointer.Bool(false) vaultConfig := &vault.ClientConfig{ CACertSecretRef: o.Spec.CACertSecretRef, @@ -92,7 +94,7 @@ func (r *VaultConnectionReconciler) Reconcile(ctx context.Context, req ctrl.Requ r.Recorder.Eventf(o, corev1.EventTypeWarning, "VaultClientError", "Failed to check Vault seal status: %s", err) errs = errors.Join(errs, err) } else { - o.Status.Valid = true + o.Status.Valid = pointer.Bool(true) } } @@ -124,7 +126,7 @@ func (r *VaultConnectionReconciler) Reconcile(ctx context.Context, req ctrl.Requ func (r *VaultConnectionReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.VaultConnection) error { logger := log.FromContext(ctx) - metrics.SetResourceStatus("vaultconnection", o, o.Status.Valid) + metrics.SetResourceStatus("vaultconnection", o, pointer.BoolDeref(o.Status.Valid, false)) if err := r.Status().Update(ctx, o); err != nil { logger.Error(err, "Failed to update the resource's status") return err diff --git a/controllers/vaultpkisecret_controller.go b/controllers/vaultpkisecret_controller.go index bad471e71..66006f8fd 100644 --- a/controllers/vaultpkisecret_controller.go +++ b/controllers/vaultpkisecret_controller.go @@ -18,6 +18,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -166,7 +167,7 @@ func (r *VaultPKISecretReconciler) Reconcile(ctx context.Context, req ctrl.Reque } // assume that status is always invalid - o.Status.Valid = false + o.Status.Valid = pointer.Bool(false) logger.Info("Must sync", "reason", syncReason) c, err := r.ClientFactory.Get(ctx, r.Client, o) if err != nil { @@ -294,7 +295,7 @@ func (r *VaultPKISecretReconciler) Reconcile(ctx context.Context, req ctrl.Reque } } - o.Status.Valid = true + o.Status.Valid = pointer.Bool(true) o.Status.Error = "" o.Status.SerialNumber = certResp.SerialNumber o.Status.Expiration = certResp.Expiration @@ -416,20 +417,20 @@ func (r *VaultPKISecretReconciler) getPath(spec secretsv1beta1.VaultPKISecretSpe return strings.Join(parts, "/") } -func (r *VaultPKISecretReconciler) recordEvent(p *secretsv1beta1.VaultPKISecret, reason, msg string, i ...interface{}) { +func (r *VaultPKISecretReconciler) recordEvent(o *secretsv1beta1.VaultPKISecret, reason, msg string, i ...interface{}) { eventType := corev1.EventTypeNormal - if !p.Status.Valid { + if !pointer.BoolDeref(o.Status.Valid, false) { eventType = corev1.EventTypeWarning } - r.Recorder.Eventf(p, eventType, reason, msg, i...) + r.Recorder.Eventf(o, eventType, reason, msg, i...) } func (r *VaultPKISecretReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.VaultPKISecret) error { logger := log.FromContext(ctx) logger.V(consts.LogLevelTrace).Info("Update status called") - metrics.SetResourceStatus("vaultpkisecret", o, o.Status.Valid) + metrics.SetResourceStatus("vaultpkisecret", o, pointer.BoolDeref(o.Status.Valid, false)) o.Status.LastGeneration = o.GetGeneration() if err := r.Status().Update(ctx, o); err != nil { diff --git a/internal/helpers/template.go b/internal/helpers/template.go index 6dcdd5f16..2dbd781af 100644 --- a/internal/helpers/template.go +++ b/internal/helpers/template.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/golang-lru/v2" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/pointer" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" secretsv1beta1 "github.com/hashicorp/vault-secrets-operator/api/v1beta1" @@ -217,7 +218,7 @@ func gatherTemplates(ctx context.Context, client ctrlclient.Client, meta *common continue } - if !obj.Status.Valid { + if !pointer.BoolDeref(obj.Status.Valid, false) { errs = errors.Join(errs, &InvalidSecretTransformationRefError{ objKey: objKey, diff --git a/internal/helpers/template_test.go b/internal/helpers/template_test.go index 181582900..63e8c82ea 100644 --- a/internal/helpers/template_test.go +++ b/internal/helpers/template_test.go @@ -13,6 +13,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/pointer" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" secretsv1beta1 "github.com/hashicorp/vault-secrets-operator/api/v1beta1" @@ -562,7 +563,7 @@ func TestNewSecretTransformationOption(t *testing.T) { if status == nil { status = &secretsv1beta1.SecretTransformationStatus{ - Valid: true, + Valid: pointer.Bool(true), } } @@ -1211,7 +1212,7 @@ func TestNewSecretTransformationOption(t *testing.T) { }, }, &secretsv1beta1.SecretTransformationStatus{ - Valid: false, + Valid: pointer.Bool(false), Error: "", }), }, From 0d03d140bd2e90a699d4eecd054a541f0b750099 Mon Sep 17 00:00:00 2001 From: Ben Ash <32777270+benashz@users.noreply.github.com> Date: Wed, 17 Jul 2024 11:47:04 -0400 Subject: [PATCH 06/10] Migrate to k8s.io/utils/ptr (#856) --- controllers/hcpauth_controller.go | 8 +- .../secrettransformation_controller.go | 8 +- controllers/vaultauth_controller.go | 12 +-- controllers/vaultconnection_controller.go | 12 ++- controllers/vaultpkisecret_controller.go | 10 +-- internal/common/common_test.go | 16 ++-- internal/helpers/hmac.go | 4 +- internal/helpers/serviceaccount.go | 4 +- internal/helpers/template.go | 4 +- internal/helpers/template_test.go | 6 +- internal/vault/cache.go | 2 +- internal/vault/cache_storage.go | 4 +- internal/vault/cache_storage_metrics_test.go | 90 +++++++++---------- test/integration/integration_test.go | 12 +-- 14 files changed, 95 insertions(+), 97 deletions(-) diff --git a/controllers/hcpauth_controller.go b/controllers/hcpauth_controller.go index a9f5632ed..2040bdc3f 100644 --- a/controllers/hcpauth_controller.go +++ b/controllers/hcpauth_controller.go @@ -13,7 +13,7 @@ import ( hvsclient "github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-secrets/preview/2023-06-13/client" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -70,10 +70,10 @@ func (r *HCPAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct logger.Error(err, "Validation failed") errs = errors.Join(err) o.Status.Error = err.Error() - o.Status.Valid = pointer.Bool(false) + o.Status.Valid = ptr.To(true) } else { o.Status.Error = "" - o.Status.Valid = pointer.Bool(true) + o.Status.Valid = ptr.To(true) } if err := r.updateStatus(ctx, o); err != nil { @@ -92,7 +92,7 @@ func (r *HCPAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct func (r *HCPAuthReconciler) updateStatus(ctx context.Context, a *secretsv1beta1.HCPAuth) error { logger := log.FromContext(ctx) - metrics.SetResourceStatus("hcpauth", a, pointer.BoolDeref(a.Status.Valid, false)) + metrics.SetResourceStatus("hcpauth", a, ptr.Deref(a.Status.Valid, false)) if err := r.Status().Update(ctx, a); err != nil { logger.Error(err, "Failed to update the resource's status") return err diff --git a/controllers/secrettransformation_controller.go b/controllers/secrettransformation_controller.go index 868abcd93..7748a4ead 100644 --- a/controllers/secrettransformation_controller.go +++ b/controllers/secrettransformation_controller.go @@ -10,7 +10,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -60,11 +60,11 @@ func (r *SecretTransformationReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, nil } - o.Status.Valid = pointer.Bool(true) + o.Status.Valid = ptr.To(true) o.Status.Error = "" errs := ValidateSecretTransformation(ctx, o) if errs != nil { - o.Status.Valid = pointer.Bool(false) + o.Status.Valid = ptr.To(false) o.Status.Error = errs.Error() logger.Error(err, "Failed to validate configured templates") r.Recorder.Eventf(o, corev1.EventTypeWarning, consts.ReasonInvalidConfiguration, @@ -80,7 +80,7 @@ func (r *SecretTransformationReconciler) Reconcile(ctx context.Context, req ctrl func (r *SecretTransformationReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.SecretTransformation) error { logger := log.FromContext(ctx) - metrics.SetResourceStatus("secrettransformation", o, pointer.BoolDeref(o.Status.Valid, false)) + metrics.SetResourceStatus("secrettransformation", o, ptr.Deref(o.Status.Valid, false)) if err := r.Status().Update(ctx, o); err != nil { logger.Error(err, "Failed to update the resource's status") return err diff --git a/controllers/vaultauth_controller.go b/controllers/vaultauth_controller.go index 65fc69cee..4bda0ba31 100644 --- a/controllers/vaultauth_controller.go +++ b/controllers/vaultauth_controller.go @@ -16,7 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -77,7 +77,7 @@ func (r *VaultAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } // assume that status is always invalid - o.Status.Valid = pointer.Bool(false) + o.Status.Valid = ptr.To(false) var errs error var conditions []metav1.Condition @@ -172,11 +172,11 @@ func (r *VaultAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( var horizon time.Duration if errs != nil { - o.Status.Valid = pointer.Bool(false) + o.Status.Valid = ptr.To(false) o.Status.Error = errs.Error() horizon = computeHorizonWithJitter(requeueDurationOnError) } else { - o.Status.Valid = pointer.Bool(true) + o.Status.Valid = ptr.To(true) o.Status.Error = "" } @@ -199,7 +199,7 @@ func (r *VaultAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( func (r *VaultAuthReconciler) recordEvent(o *secretsv1beta1.VaultAuth, reason, msg string, i ...interface{}) { eventType := corev1.EventTypeNormal - if !pointer.BoolDeref(o.Status.Valid, false) { + if !ptr.Deref(o.Status.Valid, false) { eventType = corev1.EventTypeWarning } @@ -208,7 +208,7 @@ func (r *VaultAuthReconciler) recordEvent(o *secretsv1beta1.VaultAuth, reason, m func (r *VaultAuthReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.VaultAuth, conditions ...metav1.Condition) error { logger := log.FromContext(ctx) - metrics.SetResourceStatus("vaultauth", o, pointer.BoolDeref(o.Status.Valid, false)) + metrics.SetResourceStatus("vaultauth", o, ptr.Deref(o.Status.Valid, false)) o.Status.Conditions = updateConditions(o.Status.Conditions, conditions...) if err := r.Status().Update(ctx, o); err != nil { logger.Error(err, "Failed to update the resource's status") diff --git a/controllers/vaultconnection_controller.go b/controllers/vaultconnection_controller.go index 18be093c9..6a6d4ca5c 100644 --- a/controllers/vaultconnection_controller.go +++ b/controllers/vaultconnection_controller.go @@ -7,14 +7,11 @@ import ( "context" "errors" - "k8s.io/utils/pointer" - - "github.com/hashicorp/vault-secrets-operator/internal/metrics" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -23,6 +20,7 @@ import ( secretsv1beta1 "github.com/hashicorp/vault-secrets-operator/api/v1beta1" "github.com/hashicorp/vault-secrets-operator/internal/consts" + "github.com/hashicorp/vault-secrets-operator/internal/metrics" "github.com/hashicorp/vault-secrets-operator/internal/vault" ) @@ -68,7 +66,7 @@ func (r *VaultConnectionReconciler) Reconcile(ctx context.Context, req ctrl.Requ } // assume that status is always invalid - o.Status.Valid = pointer.Bool(false) + o.Status.Valid = ptr.To(false) vaultConfig := &vault.ClientConfig{ CACertSecretRef: o.Spec.CACertSecretRef, @@ -94,7 +92,7 @@ func (r *VaultConnectionReconciler) Reconcile(ctx context.Context, req ctrl.Requ r.Recorder.Eventf(o, corev1.EventTypeWarning, "VaultClientError", "Failed to check Vault seal status: %s", err) errs = errors.Join(errs, err) } else { - o.Status.Valid = pointer.Bool(true) + o.Status.Valid = ptr.To(true) } } @@ -126,7 +124,7 @@ func (r *VaultConnectionReconciler) Reconcile(ctx context.Context, req ctrl.Requ func (r *VaultConnectionReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.VaultConnection) error { logger := log.FromContext(ctx) - metrics.SetResourceStatus("vaultconnection", o, pointer.BoolDeref(o.Status.Valid, false)) + metrics.SetResourceStatus("vaultconnection", o, ptr.Deref(o.Status.Valid, false)) if err := r.Status().Update(ctx, o); err != nil { logger.Error(err, "Failed to update the resource's status") return err diff --git a/controllers/vaultpkisecret_controller.go b/controllers/vaultpkisecret_controller.go index 66006f8fd..c52d2d400 100644 --- a/controllers/vaultpkisecret_controller.go +++ b/controllers/vaultpkisecret_controller.go @@ -18,7 +18,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -167,7 +167,7 @@ func (r *VaultPKISecretReconciler) Reconcile(ctx context.Context, req ctrl.Reque } // assume that status is always invalid - o.Status.Valid = pointer.Bool(false) + o.Status.Valid = ptr.To(false) logger.Info("Must sync", "reason", syncReason) c, err := r.ClientFactory.Get(ctx, r.Client, o) if err != nil { @@ -295,7 +295,7 @@ func (r *VaultPKISecretReconciler) Reconcile(ctx context.Context, req ctrl.Reque } } - o.Status.Valid = pointer.Bool(true) + o.Status.Valid = ptr.To(true) o.Status.Error = "" o.Status.SerialNumber = certResp.SerialNumber o.Status.Expiration = certResp.Expiration @@ -419,7 +419,7 @@ func (r *VaultPKISecretReconciler) getPath(spec secretsv1beta1.VaultPKISecretSpe func (r *VaultPKISecretReconciler) recordEvent(o *secretsv1beta1.VaultPKISecret, reason, msg string, i ...interface{}) { eventType := corev1.EventTypeNormal - if !pointer.BoolDeref(o.Status.Valid, false) { + if !ptr.Deref(o.Status.Valid, false) { eventType = corev1.EventTypeWarning } @@ -430,7 +430,7 @@ func (r *VaultPKISecretReconciler) updateStatus(ctx context.Context, o *secretsv logger := log.FromContext(ctx) logger.V(consts.LogLevelTrace).Info("Update status called") - metrics.SetResourceStatus("vaultpkisecret", o, pointer.BoolDeref(o.Status.Valid, false)) + metrics.SetResourceStatus("vaultpkisecret", o, ptr.Deref(o.Status.Valid, false)) o.Status.LastGeneration = o.GetGeneration() if err := r.Status().Update(ctx, o); err != nil { diff --git a/internal/common/common_test.go b/internal/common/common_test.go index d64afe3e9..775a767fc 100644 --- a/internal/common/common_test.go +++ b/internal/common/common_test.go @@ -18,7 +18,7 @@ import ( "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -846,7 +846,7 @@ func Test_MergeInVaultAuthGlobal(t *testing.T) { } wantK8sUnionParamsOverrideDefaultRef := wantK8sUnionParamsOverride.DeepCopy() - wantK8sUnionParamsOverrideDefaultRef.Spec.VaultAuthGlobalRef.AllowDefault = pointer.Bool(true) + wantK8sUnionParamsOverrideDefaultRef.Spec.VaultAuthGlobalRef.AllowDefault = ptr.To(true) wantK8sUnionParamsOverrideDefaultRef.Spec.VaultAuthGlobalRef.Name = "" wantK8sUnionParamsOverrideDefaultRef.Spec.VaultAuthGlobalRef.Namespace = "baz" @@ -1539,7 +1539,7 @@ func Test_MergeInVaultAuthGlobal(t *testing.T) { }, VaultAuthGlobalRef: &secretsv1beta1.VaultAuthGlobalRef{ Namespace: "baz", - AllowDefault: pointer.Bool(true), + AllowDefault: ptr.To(true), MergeStrategy: &secretsv1beta1.MergeStrategy{ Params: "union", }, @@ -1567,7 +1567,7 @@ func Test_MergeInVaultAuthGlobal(t *testing.T) { "baz": "override", }, VaultAuthGlobalRef: &secretsv1beta1.VaultAuthGlobalRef{ - AllowDefault: pointer.Bool(true), + AllowDefault: ptr.To(true), MergeStrategy: &secretsv1beta1.MergeStrategy{ Params: "union", }, @@ -1595,7 +1595,7 @@ func Test_MergeInVaultAuthGlobal(t *testing.T) { "baz": "override", }, VaultAuthGlobalRef: &secretsv1beta1.VaultAuthGlobalRef{ - AllowDefault: pointer.Bool(true), + AllowDefault: ptr.To(true), MergeStrategy: &secretsv1beta1.MergeStrategy{ Params: "union", }, @@ -1624,7 +1624,7 @@ func Test_MergeInVaultAuthGlobal(t *testing.T) { }, VaultAuthGlobalRef: &secretsv1beta1.VaultAuthGlobalRef{ Namespace: "baz", - AllowDefault: pointer.Bool(true), + AllowDefault: ptr.To(true), MergeStrategy: &secretsv1beta1.MergeStrategy{ Params: "union", }, @@ -1655,7 +1655,7 @@ func Test_MergeInVaultAuthGlobal(t *testing.T) { }, VaultAuthGlobalRef: &secretsv1beta1.VaultAuthGlobalRef{ Namespace: "baz", - AllowDefault: pointer.Bool(true), + AllowDefault: ptr.To(true), MergeStrategy: &secretsv1beta1.MergeStrategy{ Params: "union", }, @@ -1684,7 +1684,7 @@ func Test_MergeInVaultAuthGlobal(t *testing.T) { "baz": "override", }, VaultAuthGlobalRef: &secretsv1beta1.VaultAuthGlobalRef{ - AllowDefault: pointer.Bool(true), + AllowDefault: ptr.To(true), MergeStrategy: &secretsv1beta1.MergeStrategy{ Params: "union", }, diff --git a/internal/helpers/hmac.go b/internal/helpers/hmac.go index eac19a904..7461ad582 100644 --- a/internal/helpers/hmac.go +++ b/internal/helpers/hmac.go @@ -15,7 +15,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -205,7 +205,7 @@ func createHMACKeySecret(ctx context.Context, client ctrlclient.Client, objKey c Namespace: objKey.Namespace, Labels: hmacSecretLabels, }, - Immutable: pointer.Bool(true), + Immutable: ptr.To(true), Data: map[string][]byte{ HMACKeyName: key, }, diff --git a/internal/helpers/serviceaccount.go b/internal/helpers/serviceaccount.go index 38b57e4f5..aabd599a8 100644 --- a/internal/helpers/serviceaccount.go +++ b/internal/helpers/serviceaccount.go @@ -9,7 +9,7 @@ import ( v12 "k8s.io/api/authentication/v1" "k8s.io/api/core/v1" v13 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/hashicorp/vault-secrets-operator/internal/common" @@ -24,7 +24,7 @@ func RequestSAToken(ctx context.Context, client client.Client, sa *v1.ServiceAcc GenerateName: TokenGenerateName, }, Spec: v12.TokenRequestSpec{ - ExpirationSeconds: pointer.Int64(expirationSeconds), + ExpirationSeconds: ptr.To(expirationSeconds), Audiences: audiences, }, Status: v12.TokenRequestStatus{}, diff --git a/internal/helpers/template.go b/internal/helpers/template.go index 2dbd781af..b0c160b47 100644 --- a/internal/helpers/template.go +++ b/internal/helpers/template.go @@ -14,7 +14,7 @@ import ( "github.com/hashicorp/golang-lru/v2" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" secretsv1beta1 "github.com/hashicorp/vault-secrets-operator/api/v1beta1" @@ -218,7 +218,7 @@ func gatherTemplates(ctx context.Context, client ctrlclient.Client, meta *common continue } - if !pointer.BoolDeref(obj.Status.Valid, false) { + if !ptr.Deref(obj.Status.Valid, false) { errs = errors.Join(errs, &InvalidSecretTransformationRefError{ objKey: objKey, diff --git a/internal/helpers/template_test.go b/internal/helpers/template_test.go index 63e8c82ea..d49ffbc0f 100644 --- a/internal/helpers/template_test.go +++ b/internal/helpers/template_test.go @@ -13,7 +13,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" secretsv1beta1 "github.com/hashicorp/vault-secrets-operator/api/v1beta1" @@ -563,7 +563,7 @@ func TestNewSecretTransformationOption(t *testing.T) { if status == nil { status = &secretsv1beta1.SecretTransformationStatus{ - Valid: pointer.Bool(true), + Valid: ptr.To(true), } } @@ -1212,7 +1212,7 @@ func TestNewSecretTransformationOption(t *testing.T) { }, }, &secretsv1beta1.SecretTransformationStatus{ - Valid: pointer.Bool(false), + Valid: ptr.To(false), Error: "", }), }, diff --git a/internal/vault/cache.go b/internal/vault/cache.go index d2c694bbd..bd2306f83 100644 --- a/internal/vault/cache.go +++ b/internal/vault/cache.go @@ -7,7 +7,7 @@ import ( "fmt" "strings" - "github.com/hashicorp/golang-lru/v2" + lru "github.com/hashicorp/golang-lru/v2" "github.com/prometheus/client_golang/prometheus" ) diff --git a/internal/vault/cache_storage.go b/internal/vault/cache_storage.go index ec385fa30..67d74aac1 100644 --- a/internal/vault/cache_storage.go +++ b/internal/vault/cache_storage.go @@ -20,7 +20,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/json" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -231,7 +231,7 @@ func (c *defaultClientCacheStorage) Store(ctx context.Context, client ctrlclient } s := &corev1.Secret{ // we always store Clients in an Immutable secret as an anti-tampering mitigation. - Immutable: pointer.Bool(true), + Immutable: ptr.To(true), ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf(NamePrefixVCC + cacheKey.String()), Namespace: common.OperatorNamespace, diff --git a/internal/vault/cache_storage_metrics_test.go b/internal/vault/cache_storage_metrics_test.go index 442ca1ce4..7f00200a5 100644 --- a/internal/vault/cache_storage_metrics_test.go +++ b/internal/vault/cache_storage_metrics_test.go @@ -17,7 +17,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -104,38 +104,38 @@ func Test_clientCacheStorage_Metrics(t *testing.T) { storeLabelPairs := []*io_prometheus_client.LabelPair{ { - Name: pointer.String(metrics.LabelOperation), - Value: pointer.String(metrics.OperationStore), + Name: ptr.To(metrics.LabelOperation), + Value: ptr.To(metrics.OperationStore), }, } restoreLabelPairs := []*io_prometheus_client.LabelPair{ { - Name: pointer.String(metrics.LabelOperation), - Value: pointer.String(metrics.OperationRestore), + Name: ptr.To(metrics.LabelOperation), + Value: ptr.To(metrics.OperationRestore), }, } purgeLabelPairs := []*io_prometheus_client.LabelPair{ { - Name: pointer.String(metrics.LabelOperation), - Value: pointer.String(metrics.OperationPurge), + Name: ptr.To(metrics.LabelOperation), + Value: ptr.To(metrics.OperationPurge), }, } pruneLabelPairs := []*io_prometheus_client.LabelPair{ { - Name: pointer.String(metrics.LabelOperation), - Value: pointer.String(metrics.OperationPrune), + Name: ptr.To(metrics.LabelOperation), + Value: ptr.To(metrics.OperationPrune), }, } deleteLabelPairs := []*io_prometheus_client.LabelPair{ { - Name: pointer.String(metrics.LabelOperation), - Value: pointer.String(metrics.OperationDelete), + Name: ptr.To(metrics.LabelOperation), + Value: ptr.To(metrics.OperationDelete), }, } configLabelPairs := []*io_prometheus_client.LabelPair{ { - Name: pointer.String(metricsLabelEnforceEncryption), - Value: pointer.String("false"), + Name: ptr.To(metricsLabelEnforceEncryption), + Value: ptr.To("false"), }, } @@ -179,13 +179,13 @@ func Test_clientCacheStorage_Metrics(t *testing.T) { expectMetrics: expectMetrics{ store: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(5), + total: ptr.To[float64](5), labelPairs: storeLabelPairs, - errorsTotal: pointer.Float64(2), + errorsTotal: ptr.To[float64](2), }, ops: &expectedMetricVec{ - total: pointer.Float64(5), - errorsTotal: pointer.Float64(2), + total: ptr.To[float64](5), + errorsTotal: ptr.To[float64](2), labelPairs: storeLabelPairs, }, }, @@ -200,23 +200,23 @@ func Test_clientCacheStorage_Metrics(t *testing.T) { expectMetrics: expectMetrics{ store: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: storeLabelPairs, }, ops: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: storeLabelPairs, }, }, restore: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(2), - errorsTotal: pointer.Float64(4), + total: ptr.To[float64](2), + errorsTotal: ptr.To[float64](4), labelPairs: restoreLabelPairs, }, ops: &expectedMetricVec{ - total: pointer.Float64(2), - errorsTotal: pointer.Float64(4), + total: ptr.To[float64](2), + errorsTotal: ptr.To[float64](4), labelPairs: restoreLabelPairs, }, }, @@ -230,35 +230,35 @@ func Test_clientCacheStorage_Metrics(t *testing.T) { expectMetrics: expectMetrics{ store: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: storeLabelPairs, }, ops: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: storeLabelPairs, }, }, prune: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(2), - errorsTotal: pointer.Float64(1), + total: ptr.To[float64](2), + errorsTotal: ptr.To[float64](1), labelPairs: pruneLabelPairs, }, ops: &expectedMetricVec{ - total: pointer.Float64(2), - errorsTotal: pointer.Float64(1), + total: ptr.To[float64](2), + errorsTotal: ptr.To[float64](1), labelPairs: pruneLabelPairs, }, }, delete: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(2), - errorsTotal: pointer.Float64(1), + total: ptr.To[float64](2), + errorsTotal: ptr.To[float64](1), labelPairs: deleteLabelPairs, }, ops: &expectedMetricVec{ - total: pointer.Float64(2), - errorsTotal: pointer.Float64(1), + total: ptr.To[float64](2), + errorsTotal: ptr.To[float64](1), labelPairs: deleteLabelPairs, }, }, @@ -273,33 +273,33 @@ func Test_clientCacheStorage_Metrics(t *testing.T) { expectMetrics: expectMetrics{ store: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: storeLabelPairs, }, ops: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: storeLabelPairs, }, }, purge: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(1), - errorsTotal: pointer.Float64(1), + total: ptr.To[float64](1), + errorsTotal: ptr.To[float64](1), labelPairs: purgeLabelPairs, }, ops: &expectedMetricVec{ - total: pointer.Float64(1), - errorsTotal: pointer.Float64(1), + total: ptr.To[float64](1), + errorsTotal: ptr.To[float64](1), labelPairs: purgeLabelPairs, }, }, prune: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: pruneLabelPairs, }, ops: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: pruneLabelPairs, }, }, @@ -314,21 +314,21 @@ func Test_clientCacheStorage_Metrics(t *testing.T) { expectMetrics: expectMetrics{ store: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: storeLabelPairs, }, ops: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: storeLabelPairs, }, }, restore: &expected{ reqs: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: restoreLabelPairs, }, ops: &expectedMetricVec{ - total: pointer.Float64(4), + total: ptr.To[float64](4), labelPairs: restoreLabelPairs, }, }, diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index a03242d1b..6e8d5eb5e 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -45,7 +45,7 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" @@ -920,7 +920,7 @@ func createDeployment(t *testing.T, ctx context.Context, client ctrlclient.Clien "test": key.Name, }, }, - Replicas: pointer.Int32(3), + Replicas: ptr.To[int32](3), Template: corev1.PodTemplateSpec{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{ @@ -937,7 +937,7 @@ func createDeployment(t *testing.T, ctx context.Context, client ctrlclient.Clien }, }, }, - TerminationGracePeriodSeconds: pointer.Int64(2), + TerminationGracePeriodSeconds: ptr.To[int64](2), }, }, Strategy: appsv1.DeploymentStrategy{ @@ -970,7 +970,7 @@ func createArgoRolloutV1alpha1(t *testing.T, ctx context.Context, client ctrlcli "test": key.Name, }, }, - Replicas: pointer.Int32(2), + Replicas: ptr.To[int32](2), Template: corev1.PodTemplateSpec{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{ @@ -987,14 +987,14 @@ func createArgoRolloutV1alpha1(t *testing.T, ctx context.Context, client ctrlcli }, }, }, - TerminationGracePeriodSeconds: pointer.Int64(2), + TerminationGracePeriodSeconds: ptr.To[int64](2), }, }, Strategy: argorolloutsv1alpha1.RolloutStrategy{ Canary: &argorolloutsv1alpha1.CanaryStrategy{ Steps: []argorolloutsv1alpha1.CanaryStep{ { - SetWeight: pointer.Int32(50), + SetWeight: ptr.To[int32](50), }, { Pause: &argorolloutsv1alpha1.RolloutPause{ From d2eed0015bc2c6937c722fa82e39f7a8dc279245 Mon Sep 17 00:00:00 2001 From: Ben Ash <32777270+benashz@users.noreply.github.com> Date: Wed, 17 Jul 2024 11:47:39 -0400 Subject: [PATCH 07/10] Add InvalidMergeError wrapper (#855) Adds an error wrapper for common VaultAuthGlobal merge errors. --- internal/common/common.go | 84 ++++++++++++++++++++++------------ internal/common/common_test.go | 8 +++- 2 files changed, 61 insertions(+), 31 deletions(-) diff --git a/internal/common/common.go b/internal/common/common.go index 857b5b4c4..3652c576f 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -76,6 +76,18 @@ func (n *DefaultVaultAuthNotFoundError) Error() string { "default vault auth not found in namespaces=%v: global=%t", n.Namespaces, n.Global) } +type InvalidMergeError struct { + Err error +} + +func (n *InvalidMergeError) Error() string { + err := n.Err + if err == nil { + err = errors.New("unknown") + } + return fmt.Sprintf("invalid merge: %s", err) +} + func init() { var err error OperatorNamespace, err = utils.GetCurrentNamespace() @@ -288,9 +300,11 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets // object is used. if cObj.Spec.Method == "" { if gObj.Spec.DefaultAuthMethod == "" { - return nil, nil, fmt.Errorf( - "no auth method set in VaultAuth %s and no default method set in VaultAuthGlobal %s", - client.ObjectKeyFromObject(cObj), authGlobalRef) + return nil, nil, &InvalidMergeError{ + Err: fmt.Errorf( + "no auth method set in VaultAuth %s and no default method set in VaultAuthGlobal %s", + client.ObjectKeyFromObject(cObj), authGlobalRef), + } } cObj.Spec.Method = gObj.Spec.DefaultAuthMethod } @@ -304,8 +318,10 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets globalAuthMethod := gObj.Spec.Kubernetes mergeTargetAuthMethod := cObj.Spec.Kubernetes if mergeTargetAuthMethod == nil && globalAuthMethod == nil { - return nil, nil, fmt.Errorf("global auth method %s is not configured "+ - "in VaultAuthGlobal %s", cObj.Spec.Method, authGlobalRef) + return nil, nil, &InvalidMergeError{ + Err: fmt.Errorf("global auth method %s is not configured "+ + "in VaultAuthGlobal %s", cObj.Spec.Method, authGlobalRef), + } } if globalAuthMethod != nil { @@ -315,12 +331,12 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets } else { merged, err := mergeTargetAuthMethod.Merge(srcAuthMethod) if err != nil { - return nil, nil, err + return nil, nil, &InvalidMergeError{Err: err} } cObj.Spec.Kubernetes = merged } if err := cObj.Spec.Kubernetes.Validate(); err != nil { - return nil, nil, err + return nil, nil, &InvalidMergeError{Err: err} } globalAuthMount = globalAuthMethod.Mount globalAuthNamespace = globalAuthMethod.Namespace @@ -331,8 +347,10 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets globalAuthMethod := gObj.Spec.JWT mergeTargetAuthMethod := cObj.Spec.JWT if mergeTargetAuthMethod == nil && globalAuthMethod == nil { - return nil, nil, fmt.Errorf("global auth method %s is not configured "+ - "in VaultAuthGlobal %s", cObj.Spec.Method, authGlobalRef) + return nil, nil, &InvalidMergeError{ + Err: fmt.Errorf("global auth method %s is not configured "+ + "in VaultAuthGlobal %s", cObj.Spec.Method, authGlobalRef), + } } if globalAuthMethod != nil { @@ -342,12 +360,12 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets } else { merged, err := mergeTargetAuthMethod.Merge(srcAuthMethod) if err != nil { - return nil, nil, err + return nil, nil, &InvalidMergeError{Err: err} } cObj.Spec.JWT = merged } if err := cObj.Spec.JWT.Validate(); err != nil { - return nil, nil, err + return nil, nil, &InvalidMergeError{Err: err} } globalAuthMount = globalAuthMethod.Mount globalAuthNamespace = globalAuthMethod.Namespace @@ -358,8 +376,10 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets globalAuthMethod := gObj.Spec.AppRole mergeTargetAuthMethod := cObj.Spec.AppRole if mergeTargetAuthMethod == nil && globalAuthMethod == nil { - return nil, nil, fmt.Errorf("global auth method %s is not configured "+ - "in VaultAuthGlobal %s", cObj.Spec.Method, authGlobalRef) + return nil, nil, &InvalidMergeError{ + Err: fmt.Errorf("global auth method %s is not configured "+ + "in VaultAuthGlobal %s", cObj.Spec.Method, authGlobalRef), + } } if globalAuthMethod != nil { @@ -369,12 +389,12 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets } else { merged, err := mergeTargetAuthMethod.Merge(srcAuthMethod) if err != nil { - return nil, nil, err + return nil, nil, &InvalidMergeError{Err: err} } cObj.Spec.AppRole = merged } if err := cObj.Spec.AppRole.Validate(); err != nil { - return nil, nil, err + return nil, nil, &InvalidMergeError{Err: err} } globalAuthMount = globalAuthMethod.Mount globalAuthNamespace = globalAuthMethod.Namespace @@ -401,7 +421,7 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets cObj.Spec.AWS = merged } if err := cObj.Spec.AWS.Validate(); err != nil { - return nil, nil, err + return nil, nil, &InvalidMergeError{Err: err} } globalAuthMount = globalAuthMethod.Mount globalAuthNamespace = globalAuthMethod.Namespace @@ -412,8 +432,10 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets globalAuthMethod := gObj.Spec.GCP mergeTargetAuthMethod := cObj.Spec.GCP if mergeTargetAuthMethod == nil && globalAuthMethod == nil { - return nil, nil, fmt.Errorf("global auth method %s is not configured "+ - "in VaultAuthGlobal %s", cObj.Spec.Method, authGlobalRef) + return nil, nil, &InvalidMergeError{ + Err: fmt.Errorf("global auth method %s is not configured "+ + "in VaultAuthGlobal %s", cObj.Spec.Method, authGlobalRef), + } } if globalAuthMethod != nil { @@ -428,7 +450,7 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets cObj.Spec.GCP = merged } if err := cObj.Spec.GCP.Validate(); err != nil { - return nil, nil, err + return nil, nil, &InvalidMergeError{Err: err} } globalAuthMount = globalAuthMethod.Mount globalAuthNamespace = globalAuthMethod.Namespace @@ -436,19 +458,23 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets globalAuthHeaders = globalAuthMethod.Headers } default: - return nil, nil, fmt.Errorf( - "unsupported auth method %q for global auth merge", - cObj.Spec.Method, - ) + return nil, nil, &InvalidMergeError{ + Err: fmt.Errorf( + "unsupported auth method %q for global auth merge", + cObj.Spec.Method, + ), + } } cObj.Spec.Mount = firstNonZeroLen(strLenFunc, cObj.Spec.Mount, globalAuthMount, gObj.Spec.DefaultMount) if cObj.Spec.Mount == "" { - return nil, nil, fmt.Errorf( - "mount is not set in VaultAuth %s after merge with %s", - client.ObjectKeyFromObject(cObj), authGlobalRef, - ) + return nil, nil, &InvalidMergeError{ + Err: fmt.Errorf( + "mount is not set in VaultAuth %s after merge with %s", + client.ObjectKeyFromObject(cObj), authGlobalRef, + ), + } } cObj.Spec.Namespace = firstNonZeroLen(strLenFunc, @@ -473,7 +499,7 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets cObj.Spec.Params, globalAuthParams, gObj.Spec.DefaultParams) case "none": default: - return nil, nil, fmt.Errorf("unsupported params merge strategy %q", paramsMergeStrategy) + return nil, nil, &InvalidMergeError{Err: fmt.Errorf("unsupported params merge strategy %q", paramsMergeStrategy)} } switch headersMergeStrategy { @@ -484,7 +510,7 @@ func MergeInVaultAuthGlobal(ctx context.Context, c ctrlclient.Client, o *secrets cObj.Spec.Headers, globalAuthHeaders, gObj.Spec.DefaultHeaders) case "none": default: - return nil, nil, fmt.Errorf("unsupported headers merge strategy %q", headersMergeStrategy) + return nil, nil, &InvalidMergeError{Err: fmt.Errorf("unsupported headers merge strategy %q", headersMergeStrategy)} } cObj.Spec.VaultConnectionRef = firstNonZeroLen(strLenFunc, diff --git a/internal/common/common_test.go b/internal/common/common_test.go index 775a767fc..d62202312 100644 --- a/internal/common/common_test.go +++ b/internal/common/common_test.go @@ -1207,8 +1207,12 @@ func Test_MergeInVaultAuthGlobal(t *testing.T) { }, gObj: gObj.DeepCopy(), wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { - return assert.EqualError(t, err, - `unsupported auth method "invalid" for global auth merge`) + var wantErr *InvalidMergeError + if assert.ErrorAs(t, err, &wantErr) { + return assert.EqualError(t, wantErr.Err, + `unsupported auth method "invalid" for global auth merge`) + } + return false }, }, { From 5797542c8e5a476f0985525fbc2368abbde6f3e8 Mon Sep 17 00:00:00 2001 From: Ben Ash <32777270+benashz@users.noreply.github.com> Date: Wed, 17 Jul 2024 17:34:35 -0400 Subject: [PATCH 08/10] Update the VAG search order docs (#857) --- api/v1beta1/vaultauth_types.go | 11 ++++++----- chart/crds/secrets.hashicorp.com_vaultauths.yaml | 11 ++++++----- .../crd/bases/secrets.hashicorp.com_vaultauths.yaml | 11 ++++++----- docs/api/api-reference.md | 2 +- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/api/v1beta1/vaultauth_types.go b/api/v1beta1/vaultauth_types.go index 0c8c96ade..697a3f82f 100644 --- a/api/v1beta1/vaultauth_types.go +++ b/api/v1beta1/vaultauth_types.go @@ -333,11 +333,12 @@ type VaultAuthGlobalRef struct { // set on the operator's '-global-vault-auth-options' flag // // The default VaultAuthGlobal search is conditional. - // When a ref Namespace is not set, the search follows the order: - // 1. The referring VaultAuth Namespace. - // 2. The Operator's namespace. - // Otherwise, the search follows the order: - // 1. The VaultAuthGlobal ref Namespace. + // When a ref Namespace is set, the search for the default + // VaultAuthGlobal resource is constrained to that namespace. + // Otherwise, the search order is: + // 1. The default VaultAuthGlobal resource in the referring VaultAuth resource's + // namespace. + // 2. The default VaultAuthGlobal resource in the Operator's namespace. AllowDefault *bool `json:"allowDefault,omitempty"` } diff --git a/chart/crds/secrets.hashicorp.com_vaultauths.yaml b/chart/crds/secrets.hashicorp.com_vaultauths.yaml index 602266944..60d5ebb6d 100644 --- a/chart/crds/secrets.hashicorp.com_vaultauths.yaml +++ b/chart/crds/secrets.hashicorp.com_vaultauths.yaml @@ -254,11 +254,12 @@ spec: The default VaultAuthGlobal search is conditional. - When a ref Namespace is not set, the search follows the order: - 1. The referring VaultAuth Namespace. - 2. The Operator's namespace. - Otherwise, the search follows the order: - 1. The VaultAuthGlobal ref Namespace. + When a ref Namespace is set, the search for the default + VaultAuthGlobal resource is constrained to that namespace. + Otherwise, the search order is: + 1. The default VaultAuthGlobal resource in the referring VaultAuth resource's + namespace. + 2. The default VaultAuthGlobal resource in the Operator's namespace. type: boolean mergeStrategy: description: |- diff --git a/config/crd/bases/secrets.hashicorp.com_vaultauths.yaml b/config/crd/bases/secrets.hashicorp.com_vaultauths.yaml index 602266944..60d5ebb6d 100644 --- a/config/crd/bases/secrets.hashicorp.com_vaultauths.yaml +++ b/config/crd/bases/secrets.hashicorp.com_vaultauths.yaml @@ -254,11 +254,12 @@ spec: The default VaultAuthGlobal search is conditional. - When a ref Namespace is not set, the search follows the order: - 1. The referring VaultAuth Namespace. - 2. The Operator's namespace. - Otherwise, the search follows the order: - 1. The VaultAuthGlobal ref Namespace. + When a ref Namespace is set, the search for the default + VaultAuthGlobal resource is constrained to that namespace. + Otherwise, the search order is: + 1. The default VaultAuthGlobal resource in the referring VaultAuth resource's + namespace. + 2. The default VaultAuthGlobal resource in the Operator's namespace. type: boolean mergeStrategy: description: |- diff --git a/docs/api/api-reference.md b/docs/api/api-reference.md index 402bee2da..30618ae1b 100644 --- a/docs/api/api-reference.md +++ b/docs/api/api-reference.md @@ -729,7 +729,7 @@ _Appears in:_ | `name` _string_ | Name of the VaultAuthGlobal resource. | | Pattern: `^([a-z0-9.-]{1,253})$`
| | `namespace` _string_ | Namespace of the VaultAuthGlobal resource. If not provided, the namespace of
the referring VaultAuth resource is used. | | Pattern: `^([a-z0-9.-]{1,253})$`
| | `mergeStrategy` _[MergeStrategy](#mergestrategy)_ | MergeStrategy configures the merge strategy for HTTP headers and parameters
that are included in all Vault authentication requests. | | | -| `allowDefault` _boolean_ | AllowDefault when set to true will use the default VaultAuthGlobal resource
as the default if Name is not set. The 'allow-default-globals' option must be
set on the operator's '-global-vault-auth-options' flag

The default VaultAuthGlobal search is conditional.
When a ref Namespace is not set, the search follows the order:
1. The referring VaultAuth Namespace.
2. The Operator's namespace.
Otherwise, the search follows the order:
1. The VaultAuthGlobal ref Namespace. | | | +| `allowDefault` _boolean_ | AllowDefault when set to true will use the default VaultAuthGlobal resource
as the default if Name is not set. The 'allow-default-globals' option must be
set on the operator's '-global-vault-auth-options' flag

The default VaultAuthGlobal search is conditional.
When a ref Namespace is set, the search for the default
VaultAuthGlobal resource is constrained to that namespace.
Otherwise, the search order is:
1. The default VaultAuthGlobal resource in the referring VaultAuth resource's
namespace.
2. The default VaultAuthGlobal resource in the Operator's namespace. | | | #### VaultAuthGlobalSpec From dcfee8e513e198aafad467ae71ec5cdd39269282 Mon Sep 17 00:00:00 2001 From: Ben Ash <32777270+benashz@users.noreply.github.com> Date: Thu, 18 Jul 2024 16:26:37 -0400 Subject: [PATCH 09/10] CI: Bump test vault versions (#861) - Test against the latest 1.17.2 version for enterprise and community. - Test against the latest 1.16.x and 1.15.x for enterprise. --- .github/workflows/build.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index a5870d1a3..8fb713fbb 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -343,9 +343,9 @@ jobs: outputs: # JSON encoded array of k8s versions K8S_VERSIONS: '["1.30.0", "1.29.4", "1.28.9", "1.27.13", "1.26.15"]' - VAULT_N: "1.16.3" - VAULT_N_1: "1.15.9" - VAULT_N_2: "1.14.13" + VAULT_N: "1.17.2" + VAULT_N_1: "1.16.6" + VAULT_N_2: "1.15.12" latest-vault: name: vault:${{ matrix.vault-version }} kind:${{ matrix.k8s-version }} ${{ matrix.installation-method }} enterprise=${{ matrix.vault-enterprise }} needs: From ec467b06d8d9035913fea68e54b0807321c66052 Mon Sep 17 00:00:00 2001 From: Ben Ash <32777270+benashz@users.noreply.github.com> Date: Mon, 22 Jul 2024 10:36:06 -0400 Subject: [PATCH 10/10] Update changelog and versions for v0.8.0 (#860) --- CHANGELOG.md | 58 ++++++++++++++++++++++++++++++- chart/Chart.yaml | 4 +-- chart/values.yaml | 2 +- config/manager/kustomization.yaml | 2 +- 4 files changed, 61 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bc282282..97da99bc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,59 @@ +## 0.8.0 (July 18th, 2024) +**Important** + +* Helm: CRD schema changes are now automatically applied at upgrade time. + + *See [updating-crds](https://developer.hashicorp.com/vault/docs/platform/k8s/vso/installation#updating-crds-when-using-helm) for more details.* + +* This release contains CRD schema changes which remove the field validation on most VaultAuth spec fields. That means invalid VaultAuth + configurations will no longer be handled at resource application time. Please review the VSO logs and K8s + events when troubleshooting Vault authentication issues. + +Features: +* Helm: add support for auto upgrading CRDs: [GH-789](https://github.com/hashicorp/vault-secrets-operator/pull/789) +* VaultStaticSecret: support [instant event-driven updates](https://developer.hashicorp.com/vault/docs/platform/k8s/vso/sources/vault#instant-updates): [GH-771](https://github.com/hashicorp/vault-secrets-operator/pull/771) +* Add new [VaultAuthGlobal](https://developer.hashicorp.com/vault/docs/platform/k8s/vso/sources/vault#vaultauthglobal-custom-resource) type for shared VaultAuth configurations: + [GH-735](https://github.com/hashicorp/vault-secrets-operator/pull/735) + [GH-800](https://github.com/hashicorp/vault-secrets-operator/pull/800) + [GH-847](https://github.com/hashicorp/vault-secrets-operator/pull/847) + [GH-855](https://github.com/hashicorp/vault-secrets-operator/pull/855) + [GH-850](https://github.com/hashicorp/vault-secrets-operator/pull/850) +* CachingClientFactory: support client taints to trigger Vault client token validation: + [GH-717](https://github.com/hashicorp/vault-secrets-operator/pull/717) + [GH-769](https://github.com/hashicorp/vault-secrets-operator/pull/769) + +Improvements: +* VPS: add ca.crt from issuing CA for tls secret type: [GH-848](https://github.com/hashicorp/vault-secrets-operator/pull/848) +* Helm: support setting VaultAuthGlobalRef on VaultAuth: [GH-851](https://github.com/hashicorp/vault-secrets-operator/pull/851) +* Migrate to k8s.io/utils/ptr: [GH-856](https://github.com/hashicorp/vault-secrets-operator/pull/856) +* Core: update backoff option docs: [GH-801](https://github.com/hashicorp/vault-secrets-operator/pull/801) + +Fix: +* VaultAuth: set valid status on VaultAuthGlobal deref error: [GH-854](https://github.com/hashicorp/vault-secrets-operator/pull/854) +* VDS: properly handle the clone cache key variant during client callback execution: [GH-835](https://github.com/hashicorp/vault-secrets-operator/pull/835) +* Core: delete resource status metrics upon object deletion: [GH-815](https://github.com/hashicorp/vault-secrets-operator/pull/815) +* VSS: use a constant backoff on some reconciliation errors: [GH-811](https://github.com/hashicorp/vault-secrets-operator/pull/811) +* VDS: work around Vault DB static creds TTL rollover bug: [GH-730](https://github.com/hashicorp/vault-secrets-operator/pull/730) + +Build: +* CI: bump Vault versions: [GH-797](https://github.com/hashicorp/vault-secrets-operator/pull/797) + +Dependency Updates: +* Bump cloud.google.com/go/compute/metadata from 0.4.0 to 0.5.0: [GH-853](https://github.com/hashicorp/vault-secrets-operator/pull/853) +* Bump github.com/gruntwork-io/terratest from 0.46.16 to 0.47.0: [GH-852](https://github.com/hashicorp/vault-secrets-operator/pull/852) +* Bump github.com/hashicorp/go-getter from 1.7.4 to 1.7.5: [GH-834](https://github.com/hashicorp/vault-secrets-operator/pull/834) +* Bump github.com/hashicorp/go-retryablehttp from 0.7.1 to 0.7.7: [GH-833](https://github.com/hashicorp/vault-secrets-operator/pull/833) +* Bump github.com/hashicorp/go-version from 1.6.0 to 1.7.0: [GH-810](https://github.com/hashicorp/vault-secrets-operator/pull/810) +* Bump golang.org/x/crypto from 0.24.0 to 0.25.0: [GH-843](https://github.com/hashicorp/vault-secrets-operator/pull/843) +* Bump google.golang.org/api from 0.186.0 to 0.188.0: [GH-846](https://github.com/hashicorp/vault-secrets-operator/pull/846) +* Bump google.golang.org/grpc from 1.64.0 to 1.64.1: [GH-845](https://github.com/hashicorp/vault-secrets-operator/pull/845) +* Bump k8s.io/api from 0.30.1 to 0.30.2: [GH-822](https://github.com/hashicorp/vault-secrets-operator/pull/822) +* Bump k8s.io/apiextensions-apiserver from 0.30.1 to 0.30.2: [GH-828](https://github.com/hashicorp/vault-secrets-operator/pull/828) +* Bump k8s.io/client-go from 0.30.1 to 0.30.2: [GH-830](https://github.com/hashicorp/vault-secrets-operator/pull/830) +* Bump sigs.k8s.io/controller-runtime from 0.18.3 to 0.18.4: [GH-808](https://github.com/hashicorp/vault-secrets-operator/pull/808) +* Bump ubi9/ubi-micro from 9.4-6.1716471860 to 9.4-9: [GH-819](https://github.com/hashicorp/vault-secrets-operator/pull/819) +* Bump ubi9/ubi-minimal from 9.4-949.1717074713 to 9.4-1134: [GH-820](https://github.com/hashicorp/vault-secrets-operator/pull/820) + ## 0.7.1 (May 30th, 2024) Fix: @@ -31,7 +87,7 @@ Fix: * VDS: Selectively log calls to SyncRegistry.Delete(): [GH-718](https://github.com/hashicorp/vault-secrets-operator/pull/718) Build: -* CI: test against vault-1.16.2: [GH-715](https://github.com/hashicorp/vault-secrets-operator/pull/715) +* CI: Bump test vault versions: [GH-861](https://github.com/hashicorp/vault-secrets-operator/pull/861) * Bump GH actions for node 16 obsolescence: [GH-738](https://github.com/hashicorp/vault-secrets-operator/pull/738) Dependency Updates: diff --git a/chart/Chart.yaml b/chart/Chart.yaml index 20b62ea46..387dce627 100644 --- a/chart/Chart.yaml +++ b/chart/Chart.yaml @@ -3,8 +3,8 @@ apiVersion: v2 name: vault-secrets-operator -version: 0.7.1 -appVersion: "0.7.1" +version: 0.8.0 +appVersion: "0.8.0" kubeVersion: ">=1.21.0-0" description: Official Vault Secrets Operator Chart type: application diff --git a/chart/values.yaml b/chart/values.yaml index 7c002c481..90ec3a6d3 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -161,7 +161,7 @@ controller: image: pullPolicy: IfNotPresent repository: hashicorp/vault-secrets-operator - tag: 0.7.1 + tag: 0.8.0 # logging logging: diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 6df89fc64..c3033a831 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -16,4 +16,4 @@ kind: Kustomization images: - name: controller newName: hashicorp/vault-secrets-operator - newTag: 0.7.1 + newTag: 0.8.0