Skip to content

Commit

Permalink
Rework webhook validation to check storage requirements
Browse files Browse the repository at this point in the history
This commit reworks the webhook validation to validate that storage updates fulfill
the requirements (only increasing storage, using a valid storage class that allows
storage expansion).

This required moving the webhook validation into the controller package to allow use
of the k8sclient without a dependency cycle, and requires some rework in webhook
registration
  • Loading branch information
robbavey committed Feb 1, 2024
1 parent 403763e commit 86ca421
Show file tree
Hide file tree
Showing 10 changed files with 801 additions and 367 deletions.
5 changes: 3 additions & 2 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ import (
"github.com/elastic/cloud-on-k8s/v2/pkg/controller/kibana"
"github.com/elastic/cloud-on-k8s/v2/pkg/controller/license"
licensetrial "github.com/elastic/cloud-on-k8s/v2/pkg/controller/license/trial"
lsvalidation "github.com/elastic/cloud-on-k8s/v2/pkg/controller/logstash/validation"
"github.com/elastic/cloud-on-k8s/v2/pkg/controller/maps"
"github.com/elastic/cloud-on-k8s/v2/pkg/controller/remoteca"
"github.com/elastic/cloud-on-k8s/v2/pkg/controller/stackconfigpolicy"
Expand Down Expand Up @@ -1011,7 +1012,6 @@ func setupWebhook(
&kbv1.Kibana{},
&kbv1beta1.Kibana{},
&emsv1alpha1.ElasticMapsServer{},
&logstashv1alpha1.Logstash{},
&policyv1alpha1.StackConfigPolicy{},
}
for _, obj := range webhookObjects {
Expand All @@ -1027,9 +1027,10 @@ func setupWebhook(
}
}

// Elasticsearch and ElasticsearchAutoscaling validating webhooks are wired up differently, in order to access the k8s client
// Logstash, Elasticsearch and ElasticsearchAutoscaling validating webhooks are wired up differently, in order to access the k8s client
esvalidation.RegisterWebhook(mgr, params.ValidateStorageClass, exposedNodeLabels, checker, managedNamespaces)
esavalidation.RegisterWebhook(mgr, params.ValidateStorageClass, checker, managedNamespaces)
lsvalidation.RegisterWebhook(mgr, params.ValidateStorageClass, managedNamespaces)

// wait for the secret to be populated in the local filesystem before returning
interval := time.Second * 1
Expand Down
44 changes: 22 additions & 22 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -202,28 +202,6 @@ webhooks:
resources:
- kibanas
sideEffects: None
- admissionReviewVersions:
- v1
- v1beta1
clientConfig:
service:
name: webhook-service
namespace: system
path: /validate-logstash-k8s-elastic-co-v1alpha1-logstash
failurePolicy: Ignore
matchPolicy: Exact
name: elastic-logstash-validation-v1alpha1.k8s.elastic.co
rules:
- apiGroups:
- logstash.k8s.elastic.co
apiVersions:
- v1alpha1
operations:
- CREATE
- UPDATE
resources:
- logstashes
sideEffects: None
- admissionReviewVersions:
- v1
- v1beta1
Expand Down Expand Up @@ -312,3 +290,25 @@ webhooks:
resources:
- elasticsearchautoscalers
sideEffects: None
- admissionReviewVersions:
- v1
- v1beta1
clientConfig:
service:
name: webhook-service
namespace: system
path: /validate-logstash-k8s-elastic-co-v1alpha1-logstash
failurePolicy: Ignore
matchPolicy: Exact
name: elastic-logstash-validation-v1alpha1.k8s.elastic.co
rules:
- apiGroups:
- logstash.k8s.elastic.co
apiVersions:
- v1alpha1
operations:
- CREATE
- UPDATE
resources:
- logstashes
sideEffects: None
85 changes: 0 additions & 85 deletions pkg/apis/logstash/v1alpha1/webhook.go

This file was deleted.

181 changes: 0 additions & 181 deletions pkg/apis/logstash/v1alpha1/webhook_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion pkg/apis/logstash/v1alpha1/zz_generated.deepcopy.go

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

3 changes: 2 additions & 1 deletion pkg/controller/logstash/logstash_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/watches"
"github.com/elastic/cloud-on-k8s/v2/pkg/controller/logstash/labels"
"github.com/elastic/cloud-on-k8s/v2/pkg/controller/logstash/pipelines"
"github.com/elastic/cloud-on-k8s/v2/pkg/controller/logstash/validation"
"github.com/elastic/cloud-on-k8s/v2/pkg/utils/k8s"
ulog "github.com/elastic/cloud-on-k8s/v2/pkg/utils/log"
)
Expand Down Expand Up @@ -211,7 +212,7 @@ func (r *ReconcileLogstash) validate(ctx context.Context, logstash logstashv1alp
defer tracing.Span(&ctx)()

// Run create validations only as update validations require old object which we don't have here.
if _, err := logstash.ValidateCreate(); err != nil {
if err := validation.ValidateLogstash(&logstash); err != nil {
ulog.FromContext(ctx).Error(err, "Validation failed")
k8s.MaybeEmitErrorEvent(r.recorder, err, &logstash, events.EventReasonValidation, err.Error())
return tracing.CaptureError(ctx, err)
Expand Down
Loading

0 comments on commit 86ca421

Please sign in to comment.