Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Commit

Permalink
Resolve review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Avi Sharma <avi.08.sh@gmail.com>
  • Loading branch information
avi-08 committed Jun 21, 2023
1 parent 365affc commit 40052a8
Show file tree
Hide file tree
Showing 14 changed files with 130 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,12 @@ spec:
- name
type: object
type: array
serviceAccount:
description: ServiceAccount represents the service account to be used
to make requests to the API server for evaluating conditions. If
not provided, it uses the default service account of the readiness
provider controller.
serviceAccountRef:
description: ServiceAccountRef represents the service account to be
used to make requests to the API server for evaluating conditions.
If not provided, it uses the default service account of the readiness
provider controller, which may not have appropriate RBAC for evaluating
conditions.
properties:
name:
description: Name is the name of the service account to be used
Expand Down
9 changes: 5 additions & 4 deletions apis/core/v1alpha2/readinessprovider_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,16 @@ type ReadinessProviderSpec struct {
// Conditions is the set of checks that must be evaluated to true to mark the provider as ready
Conditions []ReadinessProviderCondition `json:"conditions"`

// ServiceAccount represents the service account to be used
// ServiceAccountRef represents the service account to be used
// to make requests to the API server for evaluating conditions.
// If not provided, it uses the default service account
// of the readiness provider controller.
// of the readiness provider controller,
// which may not have appropriate RBAC for evaluating conditions.
//+kubebuilder:validation:Optional
ServiceAccount *ServiceAccountSource `json:"serviceAccount"`
ServiceAccountRef *ServiceAccountRef `json:"serviceAccountRef"`
}

type ServiceAccountSource struct {
type ServiceAccountRef struct {
// Namespace is the namespace containing the service account.
Namespace string `json:"namespace"`

Expand Down
77 changes: 61 additions & 16 deletions apis/core/v1alpha2/readinessprovider_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,44 +6,88 @@ package v1alpha2
import (
"context"
"fmt"
"reflect"

corev1 "k8s.io/api/core/v1"
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/client"
"sigs.k8s.io/controller-runtime/pkg/client/config"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)

var (
// log is for logging in this package.
readinessproviderlog = logf.Log.WithName("readinessprovider-resource").WithValues("apigroup", "core")
// cli is the client used for making calls to the k8s API server.
cli client.Client
// kubeClient is the client used for making calls to the k8s API server.
kubeClient client.Client
)

// SetupWebhookWithManager adds the webhook to the manager.
func (r *ReadinessProvider) SetupWebhookWithManager(mgr ctrl.Manager) error {
cli = mgr.GetClient()
s, err := getScheme()
if err != nil {
return err
}

kubeClient, err = client.New(mgr.GetConfig(), client.Options{Scheme: s})
if err != nil {
return err
}

return ctrl.NewWebhookManagedBy(mgr).
For(r).
Complete()
}

var _ webhook.Validator = &ReadinessProvider{}

// Get a cached client.
func (r *ReadinessProvider) getClient() (client.Client, error) {
if kubeClient != nil && !reflect.ValueOf(kubeClient).IsNil() {
return kubeClient, nil
}

s, err := getScheme()
if err != nil {
return nil, err
}

cfg, err := config.GetConfig()
if err != nil {
return nil, err
}
return client.New(cfg, client.Options{Scheme: s})
}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *ReadinessProvider) ValidateCreate() error {
readinessproviderlog.Info("validate create", "name", r.Name)
return r.validateObject()
ctx := context.Background()

c, err := r.getClient()
if err != nil {
return apierrors.NewInternalError(err)
}

return r.validateObject(ctx, c)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *ReadinessProvider) ValidateUpdate(_ runtime.Object) error {
readinessproviderlog.Info("validate update", "name", r.Name)
return r.validateObject()

ctx := context.Background()

c, err := r.getClient()
if err != nil {
return apierrors.NewInternalError(err)
}

return r.validateObject(ctx, c)
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
Expand All @@ -52,35 +96,36 @@ func (r *ReadinessProvider) ValidateDelete() error {
return nil
}

func (r *ReadinessProvider) validateObject() error {
func (r *ReadinessProvider) validateObject(ctx context.Context, k8sClient client.Client) error {
var allErrors field.ErrorList
specPath := field.NewPath("spec")

saSource := r.Spec.ServiceAccount
if saSource != nil {
if saSource.Name == "" {
saRef := r.Spec.ServiceAccountRef
if saRef != nil {
if saRef.Name == "" {
allErrors = append(allErrors, field.Required(specPath.Child("serviceAccount").Child("Name"), "missing required field"))
}

if saSource.Namespace == "" {
if saRef.Namespace == "" {
allErrors = append(allErrors, field.Required(specPath.Child("serviceAccount").Child("Namespace"), "missing required field"))
}
}

// Checking if provided serviceaccount is valid
if saSource != nil && len(allErrors) == 0 {
if saRef != nil && len(allErrors) == 0 {
sa := &corev1.ServiceAccount{}
readinessproviderlog.Info("checking if service account is present", "source", saSource)
err := cli.Get(context.Background(), client.ObjectKey{
Namespace: saSource.Namespace,
Name: saSource.Name,
readinessproviderlog.Info("checking if service account is present", "source", saRef)
err := k8sClient.Get(ctx, client.ObjectKey{
Namespace: saRef.Namespace,
Name: saRef.Name,
}, sa)

if err != nil {
allErrors = append(allErrors, field.Invalid(specPath.Child("serviceAccount"), saSource, err.Error()))
allErrors = append(allErrors, field.Invalid(specPath.Child("serviceAccount"), saRef, err.Error()))
}
}

// Validate conditions
for _, condition := range r.Spec.Conditions {
if condition.ResourceExistenceCondition == nil {
allErrors = append(
Expand Down
14 changes: 7 additions & 7 deletions apis/core/v1alpha2/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 @@ -92,11 +92,12 @@ spec:
- name
type: object
type: array
serviceAccount:
description: ServiceAccount represents the service account to be used
to make requests to the API server for evaluating conditions. If
not provided, it uses the default service account of the readiness
provider controller.
serviceAccountRef:
description: ServiceAccountRef represents the service account to be
used to make requests to the API server for evaluating conditions.
If not provided, it uses the default service account of the readiness
provider controller, which may not have appropriate RBAC for evaluating
conditions.
properties:
name:
description: Name is the name of the service account to be used
Expand Down
2 changes: 1 addition & 1 deletion readiness/controller/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ require (
github.com/go-logr/logr v1.2.3
github.com/onsi/ginkgo/v2 v2.9.2
github.com/onsi/gomega v1.27.6
github.com/pkg/errors v0.9.1
github.com/vmware-tanzu/tanzu-framework/apis/core v0.0.0-00010101000000-000000000000
github.com/vmware-tanzu/tanzu-framework/capabilities/client v0.0.0-00010101000000-000000000000
github.com/vmware-tanzu/tanzu-framework/util v0.0.0-00010101000000-000000000000
Expand Down Expand Up @@ -55,6 +54,7 @@ require (
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/prometheus/client_golang v1.14.0 // indirect
github.com/prometheus/client_model v0.3.0 // indirect
github.com/prometheus/common v0.37.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions readiness/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"

corev1alpha2 "github.com/vmware-tanzu/tanzu-framework/apis/core/v1alpha2"
capabilitiesDiscovery "github.com/vmware-tanzu/tanzu-framework/capabilities/client/pkg/discovery"
capabilitiesdiscovery "github.com/vmware-tanzu/tanzu-framework/capabilities/client/pkg/discovery"
"github.com/vmware-tanzu/tanzu-framework/readiness/controller/pkg/conditions"
readinesscontroller "github.com/vmware-tanzu/tanzu-framework/readiness/controller/pkg/readiness"
readinessprovidercontroller "github.com/vmware-tanzu/tanzu-framework/readiness/controller/pkg/readinessprovider"
Expand Down Expand Up @@ -110,7 +110,7 @@ func main() {

k8sClientset := kubernetes.NewForConfigOrDie(restConfig)

clusterQueryClient, err := capabilitiesDiscovery.NewClusterQueryClientForConfig(restConfig)
clusterQueryClient, err := capabilitiesdiscovery.NewClusterQueryClientForConfig(restConfig)
if err != nil {
setupLog.Error(err, "unable to create cluster query client")
os.Exit(1)
Expand Down
8 changes: 4 additions & 4 deletions readiness/controller/pkg/conditions/resourceexistence.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import (
corev1 "k8s.io/api/core/v1"

corev1alpha2 "github.com/vmware-tanzu/tanzu-framework/apis/core/v1alpha2"
capabilitiesDiscovery "github.com/vmware-tanzu/tanzu-framework/capabilities/client/pkg/discovery"
capabilitiesdiscovery "github.com/vmware-tanzu/tanzu-framework/capabilities/client/pkg/discovery"
)

// NewResourceExistenceConditionFunc returns a function for evaluating evaluate a ResourceExistenceCondition
func NewResourceExistenceConditionFunc() func(context.Context, *capabilitiesDiscovery.ClusterQueryClient, *corev1alpha2.ResourceExistenceCondition, string) (corev1alpha2.ReadinessConditionState, string) {
return func(ctx context.Context, queryClient *capabilitiesDiscovery.ClusterQueryClient, c *corev1alpha2.ResourceExistenceCondition, conditionName string) (corev1alpha2.ReadinessConditionState, string) {
func NewResourceExistenceConditionFunc() func(context.Context, *capabilitiesdiscovery.ClusterQueryClient, *corev1alpha2.ResourceExistenceCondition, string) (corev1alpha2.ReadinessConditionState, string) {
return func(ctx context.Context, queryClient *capabilitiesdiscovery.ClusterQueryClient, c *corev1alpha2.ResourceExistenceCondition, conditionName string) (corev1alpha2.ReadinessConditionState, string) {
if c == nil {
return corev1alpha2.ConditionFailureState, "resourceExistenceCondition is not defined"
}
Expand All @@ -37,7 +37,7 @@ func NewResourceExistenceConditionFunc() func(context.Context, *capabilitiesDisc
}
}

queryObject := capabilitiesDiscovery.Object(conditionName, &resourceToFind)
queryObject := capabilitiesdiscovery.Object(conditionName, &resourceToFind)
ok, err := queryClient.PreparedQuery(queryObject)()
if err != nil {
return corev1alpha2.ConditionFailureState, err.Error()
Expand Down
Loading

0 comments on commit 40052a8

Please sign in to comment.