Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] 🌱 Introduce spec.ClusterName for ProviderServiceAccount #2623

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions apis/vmware/v1beta1/providerserviceaccount_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ import (

// ProviderServiceAccountSpec defines the desired state of ProviderServiceAccount.
type ProviderServiceAccountSpec struct {
// ClusterName is the name of the Cluster this object belongs to.
// +optional
ClusterName *string `json:"clusterName,omitempty"`

// Deprecated: Ref will be removed in a future release.
// Ref specifies the reference to the VSphereCluster for which the ProviderServiceAccount needs to be realized.
// +optional
Ref *corev1.ObjectReference `json:"ref"`

// Rules specifies the privileges that need to be granted to the service account.
Expand Down
5 changes: 5 additions & 0 deletions apis/vmware/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,14 @@ spec:
spec:
description: ProviderServiceAccountSpec defines the desired state of ProviderServiceAccount.
properties:
clusterName:
description: ClusterName is the name of the Cluster this object belongs
to.
type: string
ref:
description: Ref specifies the reference to the VSphereCluster for
which the ProviderServiceAccount needs to be realized.
description: 'Deprecated: Ref will be removed in a future release.
Ref specifies the reference to the VSphereCluster for which the
ProviderServiceAccount needs to be realized.'
properties:
apiVersion:
description: API version of the referent.
Expand Down Expand Up @@ -149,7 +154,6 @@ spec:
cluster that contains the generated service account token.
type: string
required:
- ref
- rules
- targetNamespace
- targetSecretName
Expand Down
42 changes: 42 additions & 0 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,27 @@ kind: MutatingWebhookConfiguration
metadata:
name: mutating-webhook-configuration
webhooks:
- admissionReviewVersions:
- v1beta1
clientConfig:
service:
name: webhook-service
namespace: system
path: /mutate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-providerserviceaccount
failurePolicy: Fail
matchPolicy: Equivalent
name: default.providerserviceaccount.vmware.infrastructure.cluster.x-k8s.io
rules:
- apiGroups:
- vmware.infrastructure.cluster.x-k8s.io
apiVersions:
- v1beta1
operations:
- CREATE
- UPDATE
resources:
- providerserviceaccounts
sideEffects: None
- admissionReviewVersions:
- v1beta1
clientConfig:
Expand Down Expand Up @@ -94,6 +115,27 @@ kind: ValidatingWebhookConfiguration
metadata:
name: validating-webhook-configuration
webhooks:
- admissionReviewVersions:
- v1beta1
clientConfig:
service:
name: webhook-service
namespace: system
path: /validate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-providerserviceaccount
failurePolicy: Fail
matchPolicy: Equivalent
name: validation.providerserviceaccount.vmware.infrastructure.x-k8s.io
rules:
- apiGroups:
- vmware.infrastructure.cluster.x-k8s.io
apiVersions:
- v1beta1
operations:
- CREATE
- UPDATE
resources:
- providerserviceaccounts
sideEffects: None
- admissionReviewVersions:
- v1beta1
clientConfig:
Expand Down
35 changes: 26 additions & 9 deletions controllers/serviceaccount_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,9 +582,17 @@
if pSvcAccount.DeletionTimestamp != nil {
continue
}
ref := pSvcAccount.Spec.Ref
if ref != nil && ref.Name == clusterCtx.VSphereCluster.Name {
pSvcAccounts = append(pSvcAccounts, pSvcAccount)
// take ClusterName as higher precedence temporally before Ref was deprecated.
if pSvcAccount.Spec.ClusterName != nil && *pSvcAccount.Spec.ClusterName != "" {
clusterName := *pSvcAccount.Spec.ClusterName
if clusterName == clusterCtx.VSphereCluster.Name {
pSvcAccounts = append(pSvcAccounts, pSvcAccount)
}

Check warning on line 590 in controllers/serviceaccount_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/serviceaccount_controller.go#L587-L590

Added lines #L587 - L590 were not covered by tests
} else if pSvcAccount.Spec.Ref != nil { //nolint:staticcheck
ref := pSvcAccount.Spec.Ref //nolint:staticcheck
if ref.Name == clusterCtx.VSphereCluster.Name {
pSvcAccounts = append(pSvcAccounts, pSvcAccount)
}
}
}
return pSvcAccounts, nil
Expand Down Expand Up @@ -678,11 +686,20 @@
}

func toVSphereClusterRequest(providerServiceAccount *vmwarev1.ProviderServiceAccount) []reconcile.Request {
vsphereClusterRef := providerServiceAccount.Spec.Ref
if vsphereClusterRef == nil || vsphereClusterRef.Name == "" {
return nil
}
return []reconcile.Request{
{NamespacedName: client.ObjectKey{Namespace: providerServiceAccount.Namespace, Name: vsphereClusterRef.Name}},
// take ClusterName as higher precedence temporally before Ref was deprecated.
if providerServiceAccount.Spec.ClusterName != nil && *providerServiceAccount.Spec.ClusterName != "" {
clusterName := *providerServiceAccount.Spec.ClusterName
return []reconcile.Request{
{NamespacedName: client.ObjectKey{Namespace: providerServiceAccount.Namespace, Name: clusterName}},
}

Check warning on line 694 in controllers/serviceaccount_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/serviceaccount_controller.go#L691-L694

Added lines #L691 - L694 were not covered by tests
} else if providerServiceAccount.Spec.Ref != nil { //nolint:staticcheck
vsphereClusterRef := providerServiceAccount.Spec.Ref //nolint:staticcheck
if vsphereClusterRef.Name != "" {
return []reconcile.Request{
{NamespacedName: client.ObjectKey{Namespace: providerServiceAccount.Namespace, Name: vsphereClusterRef.Name}},
}
}
}

return nil

Check warning on line 704 in controllers/serviceaccount_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/serviceaccount_controller.go#L704

Added line #L704 was not covered by tests
}
2 changes: 1 addition & 1 deletion controllers/serviceaccount_controller_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func getTestProviderServiceAccount(namespace string, vSphereCluster *vmwarev1.VS
Controller: &truePointer,
},
}
pSvcAccount.Spec.Ref = &corev1.ObjectReference{
pSvcAccount.Spec.Ref = &corev1.ObjectReference{ //nolint:staticcheck
Name: vSphereCluster.Name,
}
}
Expand Down
4 changes: 4 additions & 0 deletions internal/test/helpers/envtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@
return err
}

if err := (&webhooks.ProviderServiceAccountWebhook{}).SetupWebhookWithManager(mgr); err != nil {
return err
}

Check warning on line 184 in internal/test/helpers/envtest.go

View check run for this annotation

Codecov / codecov/patch

internal/test/helpers/envtest.go#L183-L184

Added lines #L183 - L184 were not covered by tests

return (&webhooks.VSphereFailureDomainWebhook{}).SetupWebhookWithManager(mgr)
}

Expand Down
98 changes: 98 additions & 0 deletions internal/webhooks/providerserviceaccount.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
Copyright 2024 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package webhooks

import (
"context"
"fmt"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
)

// +kubebuilder:webhook:verbs=create;update,path=/validate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-providerserviceaccount,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=vmware.infrastructure.cluster.x-k8s.io,resources=providerserviceaccounts,versions=v1beta1,name=validation.providerserviceaccount.vmware.infrastructure.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1
// +kubebuilder:webhook:verbs=create;update,path=/mutate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-providerserviceaccount,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=vmware.infrastructure.cluster.x-k8s.io,resources=providerserviceaccounts,versions=v1beta1,name=default.providerserviceaccount.vmware.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1

// ProviderServiceAccountWebhook implements a validation and defaulting webhook for ProviderServiceAccount.
type ProviderServiceAccountWebhook struct{}

var _ webhook.CustomValidator = &ProviderServiceAccountWebhook{}
var _ webhook.CustomDefaulter = &ProviderServiceAccountWebhook{}

func (webhook *ProviderServiceAccountWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).
For(&vmwarev1.ProviderServiceAccount{}).
WithValidator(webhook).
WithDefaulter(webhook).
Complete()
}

// Default implements webhook.Defaulter so a webhook will be registered for the type.
func (webhook *ProviderServiceAccountWebhook) Default(_ context.Context, _ runtime.Object) error {
return nil
}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
func (webhook *ProviderServiceAccountWebhook) ValidateCreate(_ context.Context, raw runtime.Object) (admission.Warnings, error) {
var allErrs field.ErrorList

obj, ok := raw.(*vmwarev1.ProviderServiceAccount)
if !ok {
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a ProviderServiceAccount but got a %T", raw))
}

Check warning on line 62 in internal/webhooks/providerserviceaccount.go

View check run for this annotation

Codecov / codecov/patch

internal/webhooks/providerserviceaccount.go#L61-L62

Added lines #L61 - L62 were not covered by tests

spec := obj.Spec
if spec.Ref == nil && (spec.ClusterName == nil || *spec.ClusterName == "") { //nolint:staticcheck
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "should specify Ref or ClusterName"))
if spec.ClusterName != nil && *spec.ClusterName == "" {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "ClusterName"), "ClusterName should not be empty"))
}

Check warning on line 69 in internal/webhooks/providerserviceaccount.go

View check run for this annotation

Codecov / codecov/patch

internal/webhooks/providerserviceaccount.go#L68-L69

Added lines #L68 - L69 were not covered by tests
}

return nil, aggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
func (webhook *ProviderServiceAccountWebhook) ValidateUpdate(_ context.Context, _ runtime.Object, newRaw runtime.Object) (admission.Warnings, error) {
var allErrs field.ErrorList

newTyped, ok := newRaw.(*vmwarev1.ProviderServiceAccount)
if !ok {
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a ProviderServiceAccount but got a %T", newRaw))
}

Check warning on line 82 in internal/webhooks/providerserviceaccount.go

View check run for this annotation

Codecov / codecov/patch

internal/webhooks/providerserviceaccount.go#L81-L82

Added lines #L81 - L82 were not covered by tests

spec := newTyped.Spec
if spec.Ref == nil && (spec.ClusterName == nil || *spec.ClusterName == "") { //nolint:staticcheck
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "should specify Ref or ClusterName"))
if spec.ClusterName != nil && *spec.ClusterName == "" {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "ClusterName"), "ClusterName should not be empty"))
}

Check warning on line 89 in internal/webhooks/providerserviceaccount.go

View check run for this annotation

Codecov / codecov/patch

internal/webhooks/providerserviceaccount.go#L88-L89

Added lines #L88 - L89 were not covered by tests
}

return nil, aggregateObjErrors(newTyped.GroupVersionKind().GroupKind(), newTyped.Name, allErrs)
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
func (webhook *ProviderServiceAccountWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
return nil, nil

Check warning on line 97 in internal/webhooks/providerserviceaccount.go

View check run for this annotation

Codecov / codecov/patch

internal/webhooks/providerserviceaccount.go#L96-L97

Added lines #L96 - L97 were not covered by tests
}
Loading
Loading