-
Notifications
You must be signed in to change notification settings - Fork 789
Namespace Management at Scale #9470
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
Changes from 40 commits
a430321
b3c60ae
f8f0d38
986195e
8d97ca5
a1e7701
ad98634
7456d50
e77bba8
2e7fd7e
f72628b
b468d5c
14dab40
7563b65
28a596f
939dfe9
d431072
71c06f1
d12deb1
6b5cf4d
be069b3
cb86931
0ae11b0
16f2bfe
e615a77
8b03fd7
2a4deef
eba17db
a081482
9a97bec
5c52892
92ab9d8
f614f7f
772ea76
3e50487
25448fb
eba77b3
6af77b7
8a9157a
afc2025
15121d2
c07a9d3
8873c81
07ed416
4da2395
c4c2434
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ import ( | |
| corev1 "k8s.io/api/core/v1" | ||
| policyv1 "k8s.io/api/policy/v1" | ||
| apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/labels" | ||
| "k8s.io/apimachinery/pkg/runtime/schema" | ||
| "k8s.io/apimachinery/pkg/selection" | ||
|
|
@@ -44,6 +45,7 @@ import ( | |
| "sigs.k8s.io/controller-runtime/pkg/metrics/filters" | ||
| metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" | ||
| crwebhook "sigs.k8s.io/controller-runtime/pkg/webhook" | ||
| "sigs.k8s.io/yaml" | ||
|
|
||
| "github.com/elastic/cloud-on-k8s/v3/pkg/about" | ||
| agentv1alpha1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/agent/v1alpha1" | ||
|
|
@@ -69,6 +71,7 @@ import ( | |
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/container" | ||
| commonesclient "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/esclient" | ||
| commonlicense "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/license" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/nsmatch" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/operator" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/password" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/reconciler" | ||
|
|
@@ -86,6 +89,7 @@ import ( | |
| licensetrial "github.com/elastic/cloud-on-k8s/v3/pkg/controller/license/trial" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/logstash" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/maps" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/namespace" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/packageregistry" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/remotecluster" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/stackconfigpolicy" | ||
|
|
@@ -593,7 +597,29 @@ func startOperator(ctx context.Context) error { | |
| log.Error(err, "Failed to parse managed namespaces flag") | ||
| return err | ||
| } | ||
|
|
||
| // Parse the optional namespace label selector. This field is config-file | ||
| // only (a metav1.LabelSelector object); it has no CLI flag. When set, | ||
| // the operator runs in dynamic mode: the cache is cluster-wide and a | ||
| // Matcher consulted by a predicate on every controller's watches | ||
| // evaluates the selector against Namespace labels per event. | ||
| parsedNsSelector, err := parseNSSelector(log) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| dynamicNSSelector := parsedNsSelector != nil | ||
|
|
||
| // Validate that dynamic ns selector cannot co-exist with the fixed list of managed namespaces. | ||
| if dynamicNSSelector && len(managedNamespaces) > 0 { | ||
| log.Error(errors.New("namespaces and namespaceSelector are mutually exclusive"), "Invalid configuration") | ||
| return errors.New("namespaces and namespaceSelector are mutually exclusive") | ||
| } | ||
|
|
||
| switch { | ||
| case dynamicNSSelector: | ||
| log.Info("Operator configured to manage namespaces dynamically via selector", | ||
| "selector", parsedNsSelector.String(), | ||
| "operator_namespace", operatorNamespace) | ||
| case len(managedNamespaces) == 0: | ||
| log.Info("Operator configured to manage all namespaces") | ||
| case len(managedNamespaces) == 1 && managedNamespaces[0] == operatorNamespace: | ||
|
|
@@ -611,13 +637,26 @@ func startOperator(ctx context.Context) error { | |
| return err | ||
| } | ||
|
|
||
| namespaceMatcher := nsmatch.NewNamespaceMatcher(parsedNsSelector, operatorNamespace) | ||
|
|
||
| opts.Cache = cache.Options{ | ||
| DefaultNamespaces: map[string]cache.Config{}, | ||
| DefaultTransform: cache.TransformStripManagedFields(), | ||
| ByObject: byObject, | ||
| } | ||
| for _, ns := range managedNamespaces { | ||
| opts.Cache.DefaultNamespaces[ns] = cache.Config{} | ||
| if !dynamicNSSelector { | ||
| for _, ns := range managedNamespaces { | ||
| opts.Cache.DefaultNamespaces[ns] = cache.Config{} | ||
| } | ||
| } else { | ||
| opts.NewClient = func(config *rest.Config, options client.Options) (client.Client, error) { | ||
| delegate, err := client.New(config, options) // cache-backed; options.Cache already set by mgr | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return nsmatch.NewFilterClient(delegate, namespaceMatcher), nil | ||
| } | ||
| } | ||
|
|
||
| // only expose prometheus metrics if provided a non-zero port | ||
|
|
@@ -651,6 +690,9 @@ func startOperator(ctx context.Context) error { | |
| return err | ||
| } | ||
|
|
||
| // cache can only be set after manager is created. | ||
| namespaceMatcher.SetCache(mgr.GetCache()) | ||
|
|
||
| // Retrieve globally shared CA if any | ||
| ca, err := readOptionalCA(viper.GetString(operator.CADirFlag)) | ||
| if err != nil { | ||
|
|
@@ -752,6 +794,7 @@ func startOperator(ctx context.Context) error { | |
| SetDefaultSecurityContext: setDefaultSecurityContext, | ||
| ValidateStorageClass: viper.GetBool(operator.ValidateStorageClassFlag), | ||
| Tracer: tracer, | ||
| NamespaceMatcher: namespaceMatcher, | ||
| } | ||
|
|
||
| if viper.GetBool(operator.EnableWebhookFlag) { | ||
|
|
@@ -773,7 +816,7 @@ func startOperator(ctx context.Context) error { | |
|
|
||
| disableTelemetry := viper.GetBool(operator.DisableTelemetryFlag) | ||
| telemetryInterval := viper.GetDuration(operator.TelemetryIntervalFlag) | ||
| go asyncTasks(ctx, mgr, cfg, managedNamespaces, operatorNamespace, operatorInfo, disableTelemetry, telemetryInterval, tracer, dialer) | ||
| go asyncTasks(ctx, mgr, cfg, managedNamespaces, operatorNamespace, namespaceMatcher, operatorInfo, disableTelemetry, telemetryInterval, tracer, dialer) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Medium] User-secret GC ignores
if len(managedNamespaces) == 0 {
managedNamespaces = []string{AllNamespaces}
}This means a selector-scoped operator will scan and potentially delete association user secrets across the entire cluster, including namespaces it is explicitly not configured to manage. The fix needs care:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same for the autoops garbadge collector I think. Regarding the solution, if we are about to use the direct API, then perhaps pre-seeding using the direct API in main is safer? if dynamicNSSelector {
nsList, err := clientset.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
if err != nil {
log.Error(err, "Failed to list namespaces to seed the namespace matcher")
return err
}
for i := range nsList.Items {
ns := &nsList.Items[i]
namespaceMatcher.ObserveNamespace(ns)
}
}(I had already tested that when exploring pre-seeding) This way we will have the state ready in the same resource (namespace matcher)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively, a receiver function like And call this function inside the the async task after the I think this is simpler I would go with that.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 4da2395 |
||
|
|
||
| log.Info("Starting the manager", "uuid", operatorInfo.OperatorUUID, | ||
| "namespace", operatorNamespace, "version", operatorInfo.BuildInfo.Version, | ||
|
|
@@ -828,6 +871,7 @@ func asyncTasks( | |
| cfg *rest.Config, | ||
| managedNamespaces []string, | ||
| operatorNamespace string, | ||
| namespaceMatcher *nsmatch.NamespaceMatcher, | ||
| operatorInfo about.OperatorInfo, | ||
| disableTelemetry bool, | ||
| telemetryInterval time.Duration, | ||
|
|
@@ -851,7 +895,7 @@ func asyncTasks( | |
| if !disableTelemetry { | ||
| // Start the telemetry reporter | ||
| go func() { | ||
| tr := telemetry.NewReporter(operatorInfo, mgr.GetClient(), operatorNamespace, managedNamespaces, telemetryInterval, tracer) | ||
| tr := telemetry.NewReporter(operatorInfo, mgr.GetClient(), operatorNamespace, managedNamespaces, namespaceMatcher, telemetryInterval, tracer) | ||
| tr.Start(ctx) | ||
| }() | ||
| } | ||
|
|
@@ -927,6 +971,7 @@ func registerControllers(mgr manager.Manager, params operator.Parameters, access | |
| name string | ||
| registerFunc func(manager.Manager, operator.Parameters) error | ||
| }{ | ||
| {name: "Namespace", registerFunc: namespace.Add}, | ||
| {name: "APMServer", registerFunc: apmserver.Add}, | ||
| {name: "Elasticsearch", registerFunc: elasticsearch.Add}, | ||
| {name: "ElasticsearchAutoscaling", registerFunc: autoscaling.Add}, | ||
|
|
@@ -1101,3 +1146,26 @@ func buildByObject(restrictWatchedResources bool) (map[client.Object]cache.ByObj | |
|
|
||
| return byObject, nil | ||
| } | ||
|
|
||
| func parseNSSelector(log logr.Logger) (labels.Selector, error) { | ||
| if !viper.IsSet(operator.NamespaceSelectorFlag) { | ||
| return nil, nil | ||
| } | ||
| raw := viper.Get(operator.NamespaceSelectorFlag) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's maybe register this flag properly with an explanation so that it becomes better discoverable and is mentioned in help output and docs: cmd.Flags().String(
operator.NamespaceSelectorFlag,
"",
"Label selector for dynamic namespace management (enterprise feature). "+
"Must be set via config file as a metav1.LabelSelector object "+
"(e.g. matchLabels: {eck-managed: 'true'}). "+
"Mutually exclusive with --namespaces.",
)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The intention was not to have this as a CLI flag and only as part of the YAML file. Do you think we should add it as a CLI flag as well? |
||
| yamlBytes, err := yaml.Marshal(raw) | ||
| if err != nil { | ||
| log.Error(err, "Failed to serialize namespace-selector for parsing") | ||
| return nil, err | ||
| } | ||
| var ls metav1.LabelSelector | ||
| if err := yaml.Unmarshal(yamlBytes, &ls); err != nil { | ||
| log.Error(err, "Failed to parse namespace-selector") | ||
| return nil, err | ||
| } | ||
| sel, err := metav1.LabelSelectorAsSelector(&ls) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Low] Empty selector becomes match-all. |
||
| if err != nil { | ||
| log.Error(err, "Invalid namespace-selector") | ||
| return nil, err | ||
| } | ||
| return sel, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ import ( | |
| esavalidation "github.com/elastic/cloud-on-k8s/v3/pkg/controller/autoscaling/elasticsearch/validation" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/certificates" | ||
| commonlicense "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/license" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/nsmatch" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/operator" | ||
| "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/tracing" | ||
| commonwebhook "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/webhook" | ||
|
|
@@ -79,34 +80,35 @@ func setupWebhook( | |
| ) { | ||
| manageWebhookCerts := viper.GetBool(operator.ManageWebhookCertsFlag) | ||
| if manageWebhookCerts { | ||
| if err := reconcileWebhookCertsAndAddController(ctx, mgr, params.CertRotation, clientset, tracer); err != nil { | ||
| if err := reconcileWebhookCertsAndAddController(ctx, mgr, params.CertRotation, params.NamespaceMatcher, clientset, tracer); err != nil { | ||
| log.Error(err, "unable to setup the webhook certificates") | ||
| os.Exit(1) | ||
| } | ||
| } | ||
|
|
||
| checker := commonlicense.NewLicenseChecker(mgr.GetClient(), params.OperatorNamespace) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Medium] Webhook license checks can still depend on unseeded matcher state on standby replicas. The namespace-scope part now uses Because the namespace controller/seeder are leader-election gated, non-leader replicas that still serve webhook traffic do not keep that in-memory state populated. If the valid enterprise license Secret is in a dynamically selected namespace rather than the always-managed operator namespace, a standby webhook can fail to see it and reject enterprise-gated resources even though the request namespace itself matched via cached labels. Can the webhook license checker avoid the state-based
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we change the client to a wide one (e.g. cache) then the license check might fetch license secrets that it should not. I think a proper fix for that is to enhance the license checker itself a bit. Before we move with enforcing the operator license to live in the operator namespace, let me think a bit about any possible enhancement, and I will come back.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in this 15121d2 |
||
| matcher := params.NamespaceMatcher | ||
| // setup webhooks for supported types | ||
| commonwebhook.RegisterResourceWebhook(mgr, apmv1.WebhookPath, checker, managedNamespaces, apmv1.Validate, "APM Server") | ||
| commonwebhook.RegisterResourceWebhook(mgr, apmv1beta1.WebhookPath, checker, managedNamespaces, apmv1beta1.Validate, "APM Server") | ||
| commonwebhook.RegisterResourceWebhook(mgr, beatv1beta1.WebhookPath, checker, managedNamespaces, beatv1beta1.Validate, "Beat") | ||
| commonwebhook.RegisterResourceWebhook(mgr, entv1.WebhookPath, checker, managedNamespaces, entv1.Validate, "Enterprise Search") | ||
| commonwebhook.RegisterResourceWebhook(mgr, entv1beta1.WebhookPath, checker, managedNamespaces, entv1beta1.Validate, "Enterprise Search") | ||
| commonwebhook.RegisterResourceWebhook(mgr, esv1beta1.WebhookPath, checker, managedNamespaces, esv1beta1.Validate, "Elasticsearch") | ||
| commonwebhook.RegisterResourceWebhook(mgr, kbv1.WebhookPath, checker, managedNamespaces, kbv1.Validate, "Kibana") | ||
| commonwebhook.RegisterResourceWebhook(mgr, kbv1beta1.WebhookPath, checker, managedNamespaces, kbv1beta1.Validate, "Kibana") | ||
| commonwebhook.RegisterResourceWebhook(mgr, emsv1alpha1.WebhookPath, checker, managedNamespaces, emsv1alpha1.Validate, "Elastic Maps Server") | ||
| commonwebhook.RegisterResourceWebhook(mgr, eprv1alpha1.WebhookPath, checker, managedNamespaces, eprv1alpha1.Validate, "Package Registry") | ||
| scpvalidation.RegisterWebhook(mgr, checker, managedNamespaces, params.OperatorNamespace) | ||
| commonwebhook.RegisterResourceWebhook(mgr, apmv1.WebhookPath, checker, managedNamespaces, matcher, apmv1.Validate, "APM Server") | ||
| commonwebhook.RegisterResourceWebhook(mgr, apmv1beta1.WebhookPath, checker, managedNamespaces, matcher, apmv1beta1.Validate, "APM Server") | ||
| commonwebhook.RegisterResourceWebhook(mgr, beatv1beta1.WebhookPath, checker, managedNamespaces, matcher, beatv1beta1.Validate, "Beat") | ||
| commonwebhook.RegisterResourceWebhook(mgr, entv1.WebhookPath, checker, managedNamespaces, matcher, entv1.Validate, "Enterprise Search") | ||
| commonwebhook.RegisterResourceWebhook(mgr, entv1beta1.WebhookPath, checker, managedNamespaces, matcher, entv1beta1.Validate, "Enterprise Search") | ||
| commonwebhook.RegisterResourceWebhook(mgr, esv1beta1.WebhookPath, checker, managedNamespaces, matcher, esv1beta1.Validate, "Elasticsearch") | ||
| commonwebhook.RegisterResourceWebhook(mgr, kbv1.WebhookPath, checker, managedNamespaces, matcher, kbv1.Validate, "Kibana") | ||
| commonwebhook.RegisterResourceWebhook(mgr, kbv1beta1.WebhookPath, checker, managedNamespaces, matcher, kbv1beta1.Validate, "Kibana") | ||
| commonwebhook.RegisterResourceWebhook(mgr, emsv1alpha1.WebhookPath, checker, managedNamespaces, matcher, emsv1alpha1.Validate, "Elastic Maps Server") | ||
| commonwebhook.RegisterResourceWebhook(mgr, eprv1alpha1.WebhookPath, checker, managedNamespaces, matcher, eprv1alpha1.Validate, "Package Registry") | ||
| scpvalidation.RegisterWebhook(mgr, checker, managedNamespaces, params.OperatorNamespace, matcher) | ||
|
|
||
| // Logstash, Elasticsearch v1, ElasticsearchAutoscaling, and AutoOps validating webhooks are wired up | ||
| // separately so their validators can use the API client and/or embed license checks. Elasticsearch | ||
| // v1beta1 remains in the RegisterResourceWebhook list above because it only needs a ValidateFunc. | ||
| esvalidation.RegisterWebhook(mgr, params.ValidateStorageClass, exposedNodeLabels, checker, managedNamespaces) | ||
| esavalidation.RegisterWebhook(mgr, params.ValidateStorageClass, checker, managedNamespaces) | ||
| lsvalidation.RegisterWebhook(mgr, params.ValidateStorageClass, managedNamespaces) | ||
| autoopsvalidation.RegisterWebhook(mgr, checker, managedNamespaces) | ||
| agentcontroller.RegisterWebhook(mgr, checker, managedNamespaces) | ||
| esvalidation.RegisterWebhook(mgr, params.ValidateStorageClass, exposedNodeLabels, checker, managedNamespaces, matcher) | ||
| esavalidation.RegisterWebhook(mgr, params.ValidateStorageClass, checker, managedNamespaces, matcher) | ||
| lsvalidation.RegisterWebhook(mgr, params.ValidateStorageClass, managedNamespaces, matcher) | ||
| autoopsvalidation.RegisterWebhook(mgr, checker, managedNamespaces, matcher) | ||
| agentcontroller.RegisterWebhook(mgr, checker, managedNamespaces, matcher) | ||
|
|
||
| // wait for the secret to be populated in the local filesystem before returning | ||
| interval := time.Second * 1 | ||
|
|
@@ -134,7 +136,7 @@ func setupWebhook( | |
| } | ||
| } | ||
|
|
||
| func reconcileWebhookCertsAndAddController(ctx context.Context, mgr manager.Manager, certRotation certificates.RotationParams, clientset kubernetes.Interface, tracer *apm.Tracer) error { | ||
| func reconcileWebhookCertsAndAddController(ctx context.Context, mgr manager.Manager, certRotation certificates.RotationParams, m *nsmatch.NamespaceMatcher, clientset kubernetes.Interface, tracer *apm.Tracer) error { | ||
| ctx = tracing.NewContextTransaction(ctx, tracer, tracing.ReconciliationTxType, webhook.ControllerName, nil) | ||
| defer tracing.EndContextTransaction(ctx) | ||
| log.Info("Automatic management of the webhook certificates enabled") | ||
|
|
@@ -157,5 +159,5 @@ func reconcileWebhookCertsAndAddController(ctx context.Context, mgr manager.Mana | |
| return err | ||
| } | ||
|
|
||
| return webhook.Add(mgr, webhookParams, clientset, wh, tracer) | ||
| return webhook.Add(mgr, m, webhookParams, clientset, wh, tracer) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.