Skip to content

Commit

Permalink
Introduce spec.ClusterName for ProviderServiceAccount
Browse files Browse the repository at this point in the history
Introduce spec.ClusterName for ProviderServiceAccount, also add
validation webhook for it.

Signed-off-by: Gong Zhang <[email protected]>
  • Loading branch information
zhanggbj committed Jan 17, 2024
1 parent 3559ab2 commit d13d662
Show file tree
Hide file tree
Showing 10 changed files with 324 additions and 13 deletions.
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 @@ func (r *ServiceAccountReconciler) getProviderServiceAccounts(ctx context.Contex
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 {
ref := pSvcAccount.Spec.Ref //nolint:staticcheck
if ref != nil && ref.Name == clusterCtx.VSphereCluster.Name {
pSvcAccounts = append(pSvcAccounts, pSvcAccount)
}
}
}
return pSvcAccounts, nil
Expand Down Expand Up @@ -678,11 +686,20 @@ func (r *ServiceAccountReconciler) providerServiceAccountToVSphereCluster(_ cont
}

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 @@ func NewTestEnvironment(ctx context.Context) *TestEnvironment {
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

0 comments on commit d13d662

Please sign in to comment.