Skip to content

Commit 7177085

Browse files
eljohnson92eljohnson92
committed
remove defaultLinodeClient used by webhooks in favor of always using an authenticated client
1 parent 6b2f98f commit 7177085

8 files changed

+51
-72
lines changed

‎internal/webhook/v1alpha2/linodecluster_webhook.go

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ func SetupLinodeClusterWebhookWithManager(mgr ctrl.Manager) error {
4747
Complete()
4848
}
4949

50-
// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable update and deletion validation.
5150
// +kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1alpha2-linodecluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters,verbs=create,versions=v1alpha2,name=validation.linodecluster.infrastructure.cluster.x-k8s.io,admissionReviewVersions=v1
5251

5352
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
@@ -59,19 +58,13 @@ func (r *linodeClusterValidator) ValidateCreate(ctx context.Context, obj runtime
5958
spec := cluster.Spec
6059
linodeclusterlog.Info("validate create", "name", cluster.Name)
6160

62-
var linodeclient clients.LinodeClient = defaultLinodeClient
63-
skipAPIValidation := false
61+
skipAPIValidation, linodeClient := setupClientWithCredentials(ctx, r.Client, spec.CredentialsRef,
62+
cluster.Name, cluster.GetNamespace(), linodeclusterlog)
6463

65-
// Handle credentials if provided
66-
if spec.CredentialsRef != nil {
67-
skipAPIValidation, linodeclient = setupClientWithCredentials(ctx, r.Client, spec.CredentialsRef,
68-
cluster.Name, cluster.GetNamespace(), linodeclusterlog)
69-
}
70-
71-
// TODO: instrument with tracing, might need refactor to preserve readibility
64+
// TODO: instrument with tracing, might need refactor to preserve readability
7265
var errs field.ErrorList
7366

74-
if err := r.validateLinodeClusterSpec(ctx, linodeclient, spec, skipAPIValidation); err != nil {
67+
if err := r.validateLinodeClusterSpec(ctx, linodeClient, spec, skipAPIValidation); err != nil {
7568
errs = slices.Concat(errs, err)
7669
}
7770

‎internal/webhook/v1alpha2/linodeclustertemplate_webhook.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,3 @@ func SetupLinodeClusterTemplateWebhookWithManager(mgr ctrl.Manager) error {
3333
return ctrl.NewWebhookManagedBy(mgr).For(&infrav1alpha2.LinodeClusterTemplate{}).
3434
Complete()
3535
}
36-
37-
// TODO(user): EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!

‎internal/webhook/v1alpha2/linodemachine_webhook.go

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,11 @@ func SetupLinodeMachineWebhookWithManager(mgr ctrl.Manager) error {
6565
Complete()
6666
}
6767

68-
// TODO(user): EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
69-
70-
// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable update and deletion validation.
7168
// +kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1alpha2-linodemachine,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=linodemachines,verbs=create,versions=v1alpha2,name=validation.linodemachine.infrastructure.cluster.x-k8s.io,admissionReviewVersions=v1
7269

7370
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
7471
func (r *linodeMachineValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
75-
var linodeclient clients.LinodeClient = defaultLinodeClient
7672
var errs field.ErrorList
77-
skipAPIValidation := false
7873

7974
machine, ok := obj.(*infrav1alpha2.LinodeMachine)
8075
if !ok {
@@ -83,13 +78,10 @@ func (r *linodeMachineValidator) ValidateCreate(ctx context.Context, obj runtime
8378
spec := machine.Spec
8479
linodemachinelog.Info("validate create", "name", machine.Name)
8580

86-
// Handle credentials if provided
87-
if spec.CredentialsRef != nil {
88-
skipAPIValidation, linodeclient = setupClientWithCredentials(ctx, r.Client, spec.CredentialsRef,
89-
machine.Name, machine.GetNamespace(), linodemachinelog)
90-
}
81+
skipAPIValidation, linodeClient := setupClientWithCredentials(ctx, r.Client, spec.CredentialsRef,
82+
machine.Name, machine.GetNamespace(), linodemachinelog)
9183

92-
if err := r.validateLinodeMachineSpec(ctx, linodeclient, spec, skipAPIValidation); err != nil {
84+
if err := r.validateLinodeMachineSpec(ctx, linodeClient, spec, skipAPIValidation); err != nil {
9385
errs = slices.Concat(errs, err)
9486
}
9587

‎internal/webhook/v1alpha2/linodeobjectstoragebucket_webhook.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"k8s.io/apimachinery/pkg/runtime/schema"
2727
"k8s.io/apimachinery/pkg/util/validation/field"
2828
ctrl "sigs.k8s.io/controller-runtime"
29+
"sigs.k8s.io/controller-runtime/pkg/client"
2930
logf "sigs.k8s.io/controller-runtime/pkg/log"
3031
"sigs.k8s.io/controller-runtime/pkg/webhook"
3132
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -48,7 +49,9 @@ func SetupLinodeObjectStorageBucketWebhookWithManager(mgr ctrl.Manager) error {
4849
// +kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1alpha2-linodeobjectstoragebucket,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=linodeobjectstoragebuckets,verbs=create;update,versions=v1alpha2,name=validation.linodeobjectstoragebucket.infrastructure.cluster.x-k8s.io,admissionReviewVersions=v1
4950

5051
// LinodeObjectStorageBucketCustomValidator struct is responsible for validating the LinodeObjectStorageBucket resource
51-
type LinodeObjectStorageBucketCustomValidator struct{}
52+
type LinodeObjectStorageBucketCustomValidator struct {
53+
Client client.Client
54+
}
5255

5356
var _ webhook.CustomValidator = &LinodeObjectStorageBucketCustomValidator{}
5457

@@ -59,8 +62,9 @@ func (v *LinodeObjectStorageBucketCustomValidator) ValidateCreate(ctx context.Co
5962
return nil, fmt.Errorf("expected a LinodeObjectStorageBucket object but got %T", obj)
6063
}
6164
linodeobjectstoragebucketlog.Info("validate create", "name", bucket.Name)
62-
63-
return nil, v.validateLinodeObjectStorageBucket(ctx, bucket, &defaultLinodeClient)
65+
skipAPIValidation, linodeClient := setupClientWithCredentials(ctx, v.Client, bucket.Spec.CredentialsRef,
66+
bucket.Name, bucket.GetNamespace(), linodemachinelog)
67+
return nil, v.validateLinodeObjectStorageBucket(ctx, bucket, linodeClient, skipAPIValidation)
6468
}
6569

6670
// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type LinodeObjectStorageBucket.
@@ -71,7 +75,7 @@ func (v *LinodeObjectStorageBucketCustomValidator) ValidateUpdate(ctx context.Co
7175
}
7276
linodeobjectstoragebucketlog.Info("validate update", "name", bucket.Name)
7377

74-
return nil, v.validateLinodeObjectStorageBucket(ctx, bucket, &defaultLinodeClient)
78+
return nil, nil
7579
}
7680

7781
// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type LinodeObjectStorageBucket.
@@ -86,10 +90,10 @@ func (v *LinodeObjectStorageBucketCustomValidator) ValidateDelete(ctx context.Co
8690
return nil, nil
8791
}
8892

89-
func (v *LinodeObjectStorageBucketCustomValidator) validateLinodeObjectStorageBucket(ctx context.Context, bucket *infrav1alpha2.LinodeObjectStorageBucket, client clients.LinodeClient) error {
93+
func (v *LinodeObjectStorageBucketCustomValidator) validateLinodeObjectStorageBucket(ctx context.Context, bucket *infrav1alpha2.LinodeObjectStorageBucket, linodeClient clients.LinodeClient, skipAPIValidation bool) error {
9094
var errs field.ErrorList
9195

92-
if err := v.validateLinodeObjectStorageBucketSpec(ctx, bucket, client); err != nil {
96+
if err := v.validateLinodeObjectStorageBucketSpec(ctx, bucket, linodeClient, skipAPIValidation); err != nil {
9397
errs = slices.Concat(errs, err)
9498
}
9599

@@ -101,10 +105,12 @@ func (v *LinodeObjectStorageBucketCustomValidator) validateLinodeObjectStorageBu
101105
bucket.Name, errs)
102106
}
103107

104-
func (v *LinodeObjectStorageBucketCustomValidator) validateLinodeObjectStorageBucketSpec(ctx context.Context, bucket *infrav1alpha2.LinodeObjectStorageBucket, client clients.LinodeClient) field.ErrorList {
108+
func (v *LinodeObjectStorageBucketCustomValidator) validateLinodeObjectStorageBucketSpec(ctx context.Context, bucket *infrav1alpha2.LinodeObjectStorageBucket, linodeClient clients.LinodeClient, skipAPIValidation bool) field.ErrorList {
105109
var errs field.ErrorList
106-
107-
if err := validateObjectStorageRegion(ctx, client, bucket.Spec.Region, field.NewPath("spec").Child("region")); err != nil {
110+
if skipAPIValidation {
111+
return errs
112+
}
113+
if err := validateObjectStorageRegion(ctx, linodeClient, bucket.Spec.Region, field.NewPath("spec").Child("region")); err != nil {
108114
errs = append(errs, err)
109115
}
110116

‎internal/webhook/v1alpha2/linodeobjectstoragebucket_webhook_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestValidateLinodeObjectStorageBucket(t *testing.T) {
6363
Result("success", func(ctx context.Context, mck Mock) {
6464
bucket := bucket
6565
bucket.Spec.Region = "iad"
66-
assert.NoError(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient))
66+
assert.NoError(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient, true))
6767
}),
6868
),
6969
Path(
@@ -75,7 +75,7 @@ func TestValidateLinodeObjectStorageBucket(t *testing.T) {
7575
Result("success", func(ctx context.Context, mck Mock) {
7676
bucket := bucket
7777
bucket.Spec.Region = "us-iad"
78-
assert.NoError(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient))
78+
assert.NoError(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient, true))
7979
}),
8080
),
8181
Path(
@@ -87,7 +87,7 @@ func TestValidateLinodeObjectStorageBucket(t *testing.T) {
8787
Result("success", func(ctx context.Context, mck Mock) {
8888
bucket := bucket
8989
bucket.Spec.Region = "us-iad-1"
90-
assert.NoError(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient))
90+
assert.NoError(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient, true))
9191
}),
9292
),
9393
),
@@ -98,7 +98,7 @@ func TestValidateLinodeObjectStorageBucket(t *testing.T) {
9898
Result("error", func(ctx context.Context, mck Mock) {
9999
bucket := bucket
100100
bucket.Spec.Region = "123invalid"
101-
assert.Error(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient))
101+
assert.Error(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient, false))
102102
}),
103103
),
104104
Path(
@@ -107,7 +107,7 @@ func TestValidateLinodeObjectStorageBucket(t *testing.T) {
107107
Result("error", func(ctx context.Context, mck Mock) {
108108
bucket := bucket
109109
bucket.Spec.Region = "invalid-2-2"
110-
assert.Error(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient))
110+
assert.Error(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient, false))
111111
}),
112112
),
113113
Path(
@@ -117,7 +117,7 @@ func TestValidateLinodeObjectStorageBucket(t *testing.T) {
117117
Result("error", func(ctx context.Context, mck Mock) {
118118
bucket := bucket
119119
bucket.Spec.Region = "us-1"
120-
assert.Error(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient))
120+
assert.Error(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient, false))
121121
}),
122122
),
123123
Path(
@@ -127,7 +127,7 @@ func TestValidateLinodeObjectStorageBucket(t *testing.T) {
127127
mck.LinodeClient.EXPECT().GetRegion(gomock.Any(), gomock.Any()).Return(&region, nil).AnyTimes()
128128
}),
129129
Result("error", func(ctx context.Context, mck Mock) {
130-
assert.Error(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient))
130+
assert.Error(t, objvalidator.validateLinodeObjectStorageBucket(ctx, &bucket, mck.LinodeClient, false))
131131
}),
132132
),
133133
),

‎internal/webhook/v1alpha2/linodeplacementgroup_webhook.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,12 @@ func (v *LinodePlacementGroupCustomValidator) ValidateCreate(ctx context.Context
6868
}
6969
linodeplacementgrouplog.Info("Validation for LinodePlacementGroup upon creation", "name", pg.GetName())
7070

71-
var linodeclient clients.LinodeClient = defaultLinodeClient
72-
skipAPIValidation := false
73-
74-
// Handle credentials if provided
75-
if pg.Spec.CredentialsRef != nil {
76-
skipAPIValidation, linodeclient = setupClientWithCredentials(ctx, v.Client, pg.Spec.CredentialsRef,
77-
pg.Name, pg.GetNamespace(), linodeplacementgrouplog)
78-
}
71+
skipAPIValidation, linodeClient := setupClientWithCredentials(ctx, v.Client, pg.Spec.CredentialsRef,
72+
pg.Name, pg.GetNamespace(), linodeplacementgrouplog)
7973

8074
var errs field.ErrorList
8175

82-
if err := v.validateLinodePlacementGroupSpec(ctx, linodeclient, pg.Spec, pg.Name, skipAPIValidation); err != nil {
76+
if err := v.validateLinodePlacementGroupSpec(ctx, linodeClient, pg.Spec, pg.Name, skipAPIValidation); err != nil {
8377
errs = slices.Concat(errs, err)
8478
}
8579

‎internal/webhook/v1alpha2/linodevpc_webhook.go

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ func SetupLinodeVPCWebhookWithManager(mgr ctrl.Manager) error {
8585
Complete()
8686
}
8787

88-
// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable update and deletion validation.
8988
// +kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1alpha2-linodevpc,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=linodevpcs,verbs=create,versions=v1alpha2,name=validation.linodevpc.infrastructure.cluster.x-k8s.io,admissionReviewVersions=v1
9089

9190
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
@@ -97,17 +96,11 @@ func (r *linodeVPCValidator) ValidateCreate(ctx context.Context, obj runtime.Obj
9796
spec := vpc.Spec
9897
linodevpclog.Info("validate create", "name", vpc.Name)
9998

100-
var linodeclient clients.LinodeClient = defaultLinodeClient
101-
skipAPIValidation := false
99+
skipAPIValidation, linodeClient := setupClientWithCredentials(ctx, r.Client, spec.CredentialsRef,
100+
vpc.Name, vpc.GetNamespace(), linodevpclog)
102101

103-
// Handle credentials if provided
104-
if spec.CredentialsRef != nil {
105-
skipAPIValidation, linodeclient = setupClientWithCredentials(ctx, r.Client, spec.CredentialsRef,
106-
vpc.Name, vpc.GetNamespace(), linodevpclog)
107-
}
108-
109-
// TODO: instrument with tracing, might need refactor to preserve readibility
110-
errs := r.validateLinodeVPCSpec(ctx, linodeclient, spec, skipAPIValidation)
102+
// TODO: instrument with tracing, might need refactor to preserve readability
103+
errs := r.validateLinodeVPCSpec(ctx, linodeClient, spec, skipAPIValidation)
111104

112105
if len(errs) == 0 {
113106
return nil, nil

‎internal/webhook/v1alpha2/webhook_helpers.go

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"net/http"
23+
"os"
2324
"regexp"
2425
"slices"
2526
"time"
@@ -41,14 +42,6 @@ const (
4142
defaultClientTimeout = time.Second * 10
4243
)
4344

44-
var (
45-
// defaultLinodeClient is an unauthenticated Linode client
46-
defaultLinodeClient = linodeclient.NewLinodeClientWithTracing(
47-
ptr.To(linodego.NewClient(&http.Client{Timeout: defaultClientTimeout})),
48-
linodeclient.DefaultDecorator(),
49-
)
50-
)
51-
5245
func validateRegion(ctx context.Context, linodegoclient clients.LinodeClient, id string, path *field.Path, capabilities ...string) *field.Error {
5346
region, err := linodegoclient.GetRegion(ctx, id)
5447
if err != nil {
@@ -132,14 +125,24 @@ func getCredentials(ctx context.Context, crClient clients.K8sClient, credentials
132125
return &credSecret, nil
133126
}
134127

135-
// setupClientWithCredentials configures a Linode client with credentials from a secret reference
128+
// setupClientWithCredentials configures a Linode client with credentials the LINODE_TOKEN env variable or
129+
// a secret reference if it is provided
136130
// Returns (skipAPIValidation, client) - skipAPIValidation will be true if credentials cannot be found
137131
// and API validation should be skipped
138132
func setupClientWithCredentials(ctx context.Context, crClient clients.K8sClient, credRef *corev1.SecretReference,
139133
resourceName, namespace string, logger logr.Logger) (bool, clients.LinodeClient) {
140-
linodeClient := defaultLinodeClient
134+
linodeClient := linodeclient.NewLinodeClientWithTracing(
135+
ptr.To(linodego.NewClient(&http.Client{Timeout: defaultClientTimeout})),
136+
linodeclient.DefaultDecorator(),
137+
)
138+
credName := ""
139+
apiToken := []byte(os.Getenv("LINODE_TOKEN"))
140+
var err error
141+
if credRef != nil {
142+
credName = credRef.Name
143+
apiToken, err = getCredentialDataFromRef(ctx, crClient, *credRef, namespace)
144+
}
141145

142-
apiToken, err := getCredentialDataFromRef(ctx, crClient, *credRef, namespace)
143146
if err == nil {
144147
logger.Info("creating a verified linode client for create request", "name", resourceName)
145148
linodeClient.SetToken(string(apiToken))
@@ -149,7 +152,7 @@ func setupClientWithCredentials(ctx context.Context, crClient clients.K8sClient,
149152
// Handle error cases
150153
if apierrors.IsNotFound(err) {
151154
logger.Info("credentials secret not found, skipping API validation",
152-
"name", resourceName, "secret", credRef.Name)
155+
"name", resourceName, "secret", credName)
153156
return true, linodeClient
154157
}
155158

0 commit comments

Comments
 (0)