Merge remote-tracking branch 'origin' into govet-latest

PROJECT

@@ -164,6 +164,7 @@ resources:
   version: v1alpha2
   webhooks:
     validation: true
+    defaulting: true
     webhookVersion: v1
 - api:
     crdVersion: v1

api/v1alpha2/linodeobjectstoragekey_types.go

@@ -34,6 +34,26 @@ type BucketAccessRef struct {
 	Region      string `json:"region"`
 }
 
+type GeneratedSecret struct {
+	// The name of the generated Secret. If not set, the name is formatted as "{name-of-obj-key}-obj-key".
+	// +optional
+	Name string `json:"name,omitempty"`
+	// The namespace for the generated Secret. If not set, defaults to the namespace of the LinodeObjectStorageKey.
+	// +optional
+	Namespace string `json:"namespace,omitempty"`
+	// The type of the generated Secret.
+	// +kubebuilder:validation:Enum=Opaque;addons.cluster.x-k8s.io/resource-set
+	// +kubebuilder:default=Opaque
+	// +optional
+	Type corev1.SecretType `json:"type,omitempty"`
+	// How to format the data stored in the generated Secret.
+	// It supports Go template syntax and interpolating the following values: .AccessKey, .SecretKey.
+	// If no format is supplied then a generic one is used containing the values specified.
+	// When SecretType is set to addons.cluster.x-k8s.io/resource-set, a .BucketEndpoint value is also available pointing to the location of the first bucket specified in BucketAccess.
+	// +optional
+	Format map[string]string `json:"format,omitempty"`
+}
+
 // LinodeObjectStorageKeySpec defines the desired state of LinodeObjectStorageKey
 type LinodeObjectStorageKeySpec struct {
 	// BucketAccess is the list of object storage bucket labels which can be accessed using the key
@@ -43,24 +63,26 @@ type LinodeObjectStorageKeySpec struct {
 	// CredentialsRef is a reference to a Secret that contains the credentials to use for generating access keys.
 	// If not supplied then the credentials of the controller will be used.
 	// +optional
-	CredentialsRef *corev1.SecretReference `json:"credentialsRef"`
+	CredentialsRef *corev1.SecretReference `json:"credentialsRef,omitempty"`
 
 	// KeyGeneration may be modified to trigger a rotation of the access key.
 	// +kubebuilder:default=0
 	KeyGeneration int `json:"keyGeneration"`
 
+	// GeneratedSecret configures the Secret to generate containing access key details.
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	GeneratedSecret `json:"generatedSecret"`
+
 	// SecretType instructs the controller what type of secret to generate containing access key details.
+	// Deprecated: Use generatedSecret.type.
 	// +kubebuilder:validation:Enum=Opaque;addons.cluster.x-k8s.io/resource-set
-	// +kubebuilder:default=Opaque
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	// +kubebuilder:deprecatedversion:warning="secretType deprecated by generatedSecret.type"
 	// +optional
 	SecretType corev1.SecretType `json:"secretType,omitempty"`
 
 	// SecretDataFormat instructs the controller how to format the data stored in the secret containing access key details.
-	// It supports Go template syntax and interpolating the following values: .AccessKey, .SecretKey.
-	// If no format is supplied then a generic one is used containing the values specified.
-	// When SecretType is set to addons.cluster.x-k8s.io/resource-set, a .BucketEndpoint value is also available pointing to the location of the first bucket specified in BucketAccess.
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	// Deprecated: Use generatedSecret.format.
+	// +kubebuilder:deprecatedversion:warning="secretDataFormat deprecated by generatedSecret.format"
 	// +optional
 	SecretDataFormat map[string]string `json:"secretDataFormat,omitempty"`
 }
@@ -90,10 +112,6 @@ type LinodeObjectStorageKeyStatus struct {
 	// +optional
 	LastKeyGeneration *int `json:"lastKeyGeneration,omitempty"`
 
-	// SecretName specifies the name of the Secret containing access key data.
-	// +optional
-	SecretName *string `json:"secretName,omitempty"`
-
 	// AccessKeyRef stores the ID for Object Storage key provisioned.
 	// +optional
 	AccessKeyRef *int `json:"accessKeyRef,omitempty"`

api/v1alpha2/linodeobjectstoragekey_webhook.go

@@ -40,11 +40,14 @@ func (r *LinodeObjectStorageKey) SetupWebhookWithManager(mgr ctrl.Manager) error
 		Complete()
 }
 
-// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
-//+kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1alpha2-linodeobjectstoragekey,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=linodeobjectstoragekeys,verbs=create;update,versions=v1alpha2,name=validation.linodeobjectstoragekey.infrastructure.cluster.x-k8s.io,admissionReviewVersions=v1
+// +kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1alpha2-linodeobjectstoragekey,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=linodeobjectstoragekeys,verbs=create;update,versions=v1alpha2,name=validation.linodeobjectstoragekey.infrastructure.cluster.x-k8s.io,admissionReviewVersions=v1
 
 var _ webhook.Validator = &LinodeObjectStorageKey{}
 
+// +kubebuilder:webhook:path=/mutate-infrastructure-cluster-x-k8s-io-v1alpha2-linodeobjectstoragekey,mutating=true,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=linodeobjectstoragekeys,verbs=create;update,versions=v1alpha2,name=mutation.linodeobjectstoragekey.infrastructure.cluster.x-k8s.io,admissionReviewVersions=v1
+
+var _ webhook.Defaulter = &LinodeObjectStorageKey{}
+
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (r *LinodeObjectStorageKey) ValidateCreate() (admission.Warnings, error) {
 	linodeobjectstoragekeylog.Info("validate create", "name", r.Name)
@@ -67,10 +70,10 @@ func (r *LinodeObjectStorageKey) ValidateDelete() (admission.Warnings, error) {
 func (r *LinodeObjectStorageKey) validateLinodeObjectStorageKey() (admission.Warnings, error) {
 	var errs field.ErrorList
 
-	if r.Spec.SecretType == clusteraddonsv1.ClusterResourceSetSecretType && len(r.Spec.SecretDataFormat) == 0 {
+	if r.Spec.GeneratedSecret.Type == clusteraddonsv1.ClusterResourceSetSecretType && len(r.Spec.GeneratedSecret.Format) == 0 {
 		errs = append(errs, field.Invalid(
-			field.NewPath("spec").Child("secretDataFormat"),
-			r.Spec.SecretDataFormat,
+			field.NewPath("spec").Child("generatedSecret").Child("format"),
+			r.Spec.GeneratedSecret.Format,
 			fmt.Sprintf("must not be empty with Secret type %s", clusteraddonsv1.ClusterResourceSetSecretType),
 		))
 	}
@@ -81,3 +84,26 @@ func (r *LinodeObjectStorageKey) validateLinodeObjectStorageKey() (admission.War
 
 	return nil, nil
 }
+
+const defaultKeySecretNameTemplate = "%s-obj-key"
+
+// Default implements webhook.Defaulter so a webhook will be registered for the type
+func (r *LinodeObjectStorageKey) Default() {
+	linodeobjectstoragekeylog.Info("default", "name", r.Name)
+
+	// Default name and namespace derived from object metadata.
+	if r.Spec.GeneratedSecret.Name == "" {
+		r.Spec.GeneratedSecret.Name = fmt.Sprintf(defaultKeySecretNameTemplate, r.Name)
+	}
+	if r.Spec.GeneratedSecret.Namespace == "" {
+		r.Spec.GeneratedSecret.Namespace = r.Namespace
+	}
+
+	// Support deprecated fields when specified and updated fields are empty.
+	if r.Spec.SecretType != "" && r.Spec.GeneratedSecret.Type == "" {
+		r.Spec.GeneratedSecret.Type = r.Spec.SecretType
+	}
+	if len(r.Spec.SecretDataFormat) > 0 && len(r.Spec.GeneratedSecret.Format) == 0 {
+		r.Spec.GeneratedSecret.Format = r.Spec.SecretDataFormat
+	}
+}

api/v1alpha2/linodeobjectstoragekey_webhook_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusteraddonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
 )
 
@@ -36,24 +37,30 @@ func TestValidateLinodeObjectStorageKey(t *testing.T) {
 		{
 			name: "opaque",
 			spec: LinodeObjectStorageKeySpec{
-				SecretType: corev1.SecretTypeOpaque,
+				GeneratedSecret: GeneratedSecret{
+					Type: corev1.SecretTypeOpaque,
+				},
 			},
 			err: nil,
 		},
 		{
 			name: "resourceset with empty secret data format",
 			spec: LinodeObjectStorageKeySpec{
-				SecretType:       clusteraddonsv1.ClusterResourceSetSecretType,
-				SecretDataFormat: map[string]string{},
+				GeneratedSecret: GeneratedSecret{
+					Type:   clusteraddonsv1.ClusterResourceSetSecretType,
+					Format: map[string]string{},
+				},
 			},
 			err: errors.New("must not be empty with Secret type"),
 		},
 		{
 			name: "valid resourceset",
 			spec: LinodeObjectStorageKeySpec{
-				SecretType: clusteraddonsv1.ClusterResourceSetSecretType,
-				SecretDataFormat: map[string]string{
-					"file.yaml": "kind: Secret",
+				GeneratedSecret: GeneratedSecret{
+					Type: clusteraddonsv1.ClusterResourceSetSecretType,
+					Format: map[string]string{
+						"file.yaml": "kind: Secret",
+					},
 				},
 			},
 			err: nil,
@@ -84,3 +91,44 @@ func TestValidateLinodeObjectStorageKey(t *testing.T) {
 		})
 	}
 }
+
+func TestLinodeObjectStorageKeyDefault(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name              string
+		genSecret         GeneratedSecret
+		expectedName      string
+		expectedNamespace string
+	}{
+		{"already set", GeneratedSecret{Name: "secret", Namespace: "ns"}, "secret", "ns"},
+		{"no name", GeneratedSecret{Namespace: "ns"}, "key-obj-key", "ns"},
+		{"no namespace", GeneratedSecret{Name: "secret"}, "secret", "keyns"},
+	}
+
+	for _, tt := range tests {
+		testcase := tt
+
+		t.Run(testcase.name, func(t *testing.T) {
+			t.Parallel()
+
+			key := &LinodeObjectStorageKey{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "key",
+					Namespace: "keyns",
+				},
+				Spec: LinodeObjectStorageKeySpec{
+					GeneratedSecret: testcase.genSecret,
+				},
+			}
+
+			key.Default()
+			if key.Spec.GeneratedSecret.Name != testcase.expectedName {
+				t.Errorf("name: expected %s but got %s", testcase.expectedName, key.Spec.GeneratedSecret.Name)
+			}
+			if key.Spec.GeneratedSecret.Namespace != testcase.expectedNamespace {
+				t.Errorf("name: expected %s but got %s", testcase.expectedNamespace, key.Spec.GeneratedSecret.Namespace)
+			}
+		})
+	}
+}

cloud/scope/object_storage_key.go

@@ -99,16 +99,13 @@ func (s *ObjectStorageKeyScope) AddFinalizer(ctx context.Context) error {
 	return nil
 }
 
-const accessKeySecretNameTemplate = "%s-obj-key"
-
 // GenerateKeySecret returns a secret suitable for submission to the Kubernetes API.
 // The secret is expected to contain keys for accessing the bucket, as well as owner and controller references.
 func (s *ObjectStorageKeyScope) GenerateKeySecret(ctx context.Context, key *linodego.ObjectStorageKey) (*corev1.Secret, error) {
 	if key == nil {
 		return nil, errors.New("expected non-nil object storage key")
 	}
 
-	secretName := fmt.Sprintf(accessKeySecretNameTemplate, s.Key.Name)
 	secretStringData := make(map[string]string)
 
 	tmplData := map[string]string{
@@ -118,7 +115,7 @@ func (s *ObjectStorageKeyScope) GenerateKeySecret(ctx context.Context, key *lino
 
 	// If the desired secret is of ClusterResourceSet type, encapsulate the secret.
 	// Bucket details are retrieved from the first referenced LinodeObjectStorageBucket in the access key.
-	if s.Key.Spec.SecretType == clusteraddonsv1.ClusterResourceSetSecretType {
+	if s.Key.Spec.GeneratedSecret.Type == clusteraddonsv1.ClusterResourceSetSecretType {
 		// This should never run since the CRD has a validation marker to ensure bucketAccess has at least one item.
 		if len(s.Key.Spec.BucketAccess) == 0 {
 			return nil, fmt.Errorf("unable to generate %s; spec.bucketAccess must not be empty", clusteraddonsv1.ClusterResourceSetSecretType)
@@ -131,14 +128,14 @@ func (s *ObjectStorageKeyScope) GenerateKeySecret(ctx context.Context, key *lino
 		}
 
 		tmplData["BucketEndpoint"] = bucket.Hostname
-	} else if len(s.Key.Spec.SecretDataFormat) == 0 {
+	} else if len(s.Key.Spec.GeneratedSecret.Format) == 0 {
 		secretStringData = map[string]string{
 			"access_key": key.AccessKey,
 			"secret_key": key.SecretKey,
 		}
 	}
 
-	for key, tmpl := range s.Key.Spec.SecretDataFormat {
+	for key, tmpl := range s.Key.Spec.GeneratedSecret.Format {
 		goTmpl, err := template.New(key).Parse(tmpl)
 		if err != nil {
 			return nil, fmt.Errorf("unable to generate secret; failed to parse template in secret data format for key %s: %w", key, err)
@@ -154,19 +151,19 @@ func (s *ObjectStorageKeyScope) GenerateKeySecret(ctx context.Context, key *lino
 
 	secret := corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      secretName,
-			Namespace: s.Key.Namespace,
+			Name:      s.Key.Spec.GeneratedSecret.Name,
+			Namespace: s.Key.Spec.GeneratedSecret.Namespace,
 		},
-		Type:       s.Key.Spec.SecretType,
+		Type:       s.Key.Spec.GeneratedSecret.Type,
 		StringData: secretStringData,
 	}
 
-	scheme := s.Client.Scheme()
-	if err := controllerutil.SetOwnerReference(s.Key, &secret, scheme); err != nil {
-		return nil, fmt.Errorf("could not set owner ref on access key secret %s: %w", secretName, err)
-	}
-	if err := controllerutil.SetControllerReference(s.Key, &secret, scheme); err != nil {
-		return nil, fmt.Errorf("could not set controller ref on access key secret %s: %w", secretName, err)
+	// Set an owner reference on a Secret if it will exist in the same namespace as the Key resource.
+	// Kubernetes does not allow cross-namespace ownership so modifications to a Secret in another namespace won't trigger reconciliation.
+	if s.Key.Spec.GeneratedSecret.Namespace == s.Key.Namespace {
+		if err := controllerutil.SetControllerReference(s.Key, &secret, s.Client.Scheme()); err != nil {
+			return nil, fmt.Errorf("could not set controller ref on access key secret %s/%s: %w", s.Key.Spec.GeneratedSecret.Name, s.Key.Spec.GeneratedSecret.Namespace, err)
+		}
 	}
 
 	return &secret, nil