Reconfigure Linseed and linseed user when migrating from multi-index to single-index - #5132
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the operator’s log-storage reconciliation path for Calico Cloud index migration, shifting Elasticsearch user provisioning responsibilities and reconfiguring Linseed to use the correct single-index backend and index base names during migration.
Changes:
- Add an
IndexMigration/UseSingleIndexsignal from bootstrap config through controller options into rendering logic. - Disable es-kube-controllers’
elasticsearchconfigurationcontroller during index migration and move user provisioning to the operator’s log-storage users controller. - Add Cloud single-index base index naming defaults and ensure deterministic ordering to avoid reconcile churn.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/render/logstorage/linseed/linseed.go | Adds single-index backend env var rendering for single-tenant migration path. |
| pkg/render/kubecontrollers/kube-controllers.go | Adds IndexMigration flag and gates elasticsearchconfiguration controller enablement. |
| pkg/render/kubecontrollers/kube-controllers_test.go | Adds test asserting elasticsearchconfiguration is disabled during migration. |
| pkg/render/common/cloudconfig/cloudconfig.go | Extends ToTenant to optionally populate standard single-index base names deterministically. |
| pkg/render/common/cloudconfig/cloudconfig_test.go | Adds tests for ToTenant(useSingleIndex) behavior and stable ordering. |
| pkg/controller/utils/elasticsearch.go | Adds single-tenant username formatting and single-tenant Linseed/Dashboards user helpers. |
| pkg/controller/policyrecommendation/policyrecommendation_controller.go | Updates CloudConfig ToTenant call signature. |
| pkg/controller/options/options.go | Adds IndexMigration option plumbed from bootstrap config. |
| pkg/controller/manager/manager_controller_cloud.go | Updates CloudConfig ToTenant call signature. |
| pkg/controller/logstorage/users/users_controller.go | Enables operator user provisioning for single-tenant index migration; adds single-tenant user naming and credential “re-pointing”. |
| pkg/controller/logstorage/users/users_controller_test.go | Adds controller tests for single-tenant migrating behavior and credential repointing. |
| pkg/controller/logstorage/linseed/linseed_controller.go | Plumbs index-migration flag into Linseed render config and CloudConfig tenant conversion. |
| pkg/controller/logstorage/kubecontrollers/es_kube_controllers.go | Plumbs IndexMigration through controller state. |
| pkg/controller/logstorage/kubecontrollers/cloud.go | Sets kube-controllers render config IndexMigration during Cloud reconcile. |
| pkg/controller/logstorage/initializer/conditions_controller.go | Avoids no-op status writes, handles conflicts, and stabilizes condition ordering; includes users controller condition in migration. |
| pkg/controller/logstorage/dashboards/dashboards_controller.go | Updates CloudConfig ToTenant call signature and clarifies credential provisioning during migration. |
| pkg/controller/intrusiondetection/intrusiondetection_controller.go | Updates CloudConfig ToTenant call signature. |
| pkg/controller/compliance/compliance_controller.go | Updates CloudConfig ToTenant call signature. |
| pkg/common/discovery/discovery.go | Adds INDEX_MIGRATION bootstrap config parsing helper. |
| cmd/main.go | Reads INDEX_MIGRATION bootstrap config and sets controller options. |
| api/v1/tenant_types.go | Adds Cloud standard base index name mapping by datatype. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
pkg/render/common/cloudconfig/cloudconfig.go:40
- cloudStandardIndices uses a ComplianceBenchmarks base index name ("calico_compliance_benchmarks_results_standard") that doesn’t match the standard single-index name used elsewhere (e.g. Linseed rendering tests expect "calico_compliance_benchmarks_standard"). This will cause the generated Tenant indices/env vars to point Linseed (and user RBAC derived from these indices) at the wrong index base name.
v1.DataTypeComplianceBenchmarks: "calico_compliance_benchmarks_results_standard",
pkg/render/common/cloudconfig/cloudconfig.go:48
- cloudStandardIndices threat-feeds base index names (DomainSet/IPSet) don’t match the standard names used elsewhere (e.g. Linseed rendering tests expect "calico_threat_feeds_domain_set_standard" and "calico_threat_feeds_ip_set_standard"). If these names are wrong, Linseed/user RBAC will be configured for indices that don’t exist.
v1.DataTypeThreatFeedsDomainSet: "calico_threatfeeds_domainnameset_standard",
v1.DataTypeThreatFeedsIPSet: "calico_threatfeeds_ipset_standard",
pkg/controller/logstorage/users/users_controller.go:370
- clusterID() hard-codes the operator namespace as "tigera-operator". Use common.OperatorNamespace() (already used elsewhere in this controller) so this continues to work if the operator namespace is overridden.
clusterIDConfigMapKey := client.ObjectKey{Name: "cluster-info", Namespace: "tigera-operator"}
6913b70 to
cba9c23
Compare
caseydavenport
left a comment
There was a problem hiding this comment.
Just a few thoughts, mostly nits!
| // UseSingleIndex returns true if this cluster is in the last phase of a migration to single-index | ||
| // storage, during which the operator must reconfigure Linseed to use the single-index names. | ||
| func UseSingleIndex(config *corev1.ConfigMap) bool { |
There was a problem hiding this comment.
The comment suggests that this function is only applicable for migration, but the name of the function sounds more general. Perhaps this would be better named "SingleIndexMigrationDone" or similar? Or is the comment just overly specific?
There was a problem hiding this comment.
We will need to leave this on even after the migration is done. Will be turned off once we migrate them to a tenant.
| // Skip the write if nothing changed. This controller watches LogStorage, so a no-op write would | ||
| // re-trigger it and spin: each reconcile would bump the resourceVersion and enqueue another one. | ||
| if equality.Semantic.DeepEqual(ls.Status.Conditions, conditions) { | ||
| return reconcile.Result{}, nil | ||
| } |
There was a problem hiding this comment.
This seems unnecessary if we're doing the sorting below - the k8s API server will notice that nothing has changed, and won't update the generation nor send an update. So I thinkwe can remove this check (not that it's a problem really, but it's another place something could go wrong if the equality check is not correct).
| // UseSingleIndex is enabled in the last phase of an index migration for a single tenant cluster, | ||
| // during which the operator reconfigures log storage to use the single-index names. | ||
| UseSingleIndex bool |
There was a problem hiding this comment.
Is this only used in migration?
There was a problem hiding this comment.
We will keep this on until we migrate traffic to the MT environment.
2fc8c40 to
b810af8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (4)
pkg/render/logstorage/linseed/linseed.go:434
- The new UseSingleIndex branch changes Linseed's rendered env vars (BACKEND + index base-name env vars) for single-tenant clusters, but there is no corresponding test case covering this behavior in pkg/render/logstorage/linseed/linseed_test.go.
} else if l.cfg.UseSingleIndex {
// For single-tenant clusters migrating to single-index storage,
// use the elastic-single-index backend and configure index base names.
envVars = append(envVars, corev1.EnvVar{Name: "BACKEND", Value: "elastic-single-index"})
for _, index := range l.cfg.Tenant.Spec.Indices {
pkg/controller/logstorage/users/users_controller.go:139
- The watch error returned here doesn't include which ConfigMap was being watched, which makes debugging harder when this fails.
if err = utils.AddConfigMapWatch(c, configMap, common.OperatorNamespace(), &handler.EnqueueRequestForObject{}); err != nil {
return fmt.Errorf("log-storage-user-controller failed to watch the ConfigMap resource: %w", err)
}
pkg/render/common/cloudconfig/cloudconfig.go:69
- This base index name looks inconsistent with the standard single-index names used elsewhere (e.g., pkg/render/logstorage/linseed/linseed_test.go uses "calico_compliance_benchmarks_standard"). If this value is wrong, Linseed and provisioned ES roles will point at non-existent indices after migration.
v1.DataTypeComplianceBenchmarks: "calico_compliance_benchmarks_results_standard",
pkg/render/common/cloudconfig/cloudconfig.go:77
- These ThreatFeeds base index names don't match the standard names used in the existing Linseed rendering expectations (see pkg/render/logstorage/linseed/linseed_test.go, which uses "calico_threat_feeds_domain_set_standard" / "calico_threat_feeds_ip_set_standard"). A mismatch here would configure Linseed and ES RBAC for the wrong index aliases.
v1.DataTypeThreatFeedsDomainSet: "calico_threatfeeds_domainnameset_standard",
v1.DataTypeThreatFeedsIPSet: "calico_threatfeeds_ipset_standard",
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (2)
pkg/controller/logstorage/users/users_controller.go:286
- When utils.GetClusterID fails because the cluster-info ConfigMap is not found (common during early startup), returning the error will trigger controller-runtime's exponential backoff and can delay recovery. Treat NotFound as a not-ready condition (no error return) and rely on the ConfigMap watch/periodic reconcile to requeue once it appears.
clusterID, err := utils.GetClusterID(ctx, r.client)
if err != nil {
r.status.SetDegraded(operatorv1.ResourceReadError, "Waiting for the cluster ID to be available", err, reqLogger)
return reconcile.Result{}, err
}
pkg/controller/logstorage/users/users_controller.go:129
- In multi-tenant mode, this controller depends on the cluster-info ConfigMap for cluster ID (via utils.GetClusterID), but it doesn't watch that ConfigMap. If the ConfigMap is created/updated after controller startup, reconcile will only pick it up on periodic reconcile/backoff, delaying readiness and potentially increasing error churn. Add an explicit watch for the cluster-info ConfigMap in the multi-tenant watch setup.
This issue also appears on line 282 of the same file.
if opts.MultiTenant {
if err = c.WatchObject(&operatorv1.Tenant{}, &handler.EnqueueRequestForObject{}); err != nil {
return fmt.Errorf("log-storage-user-controller failed to watch Tenant resource: %w", err)
}
| } else if l.cfg.UseSingleIndex { | ||
| // For single-tenant clusters migrating to single-index storage, | ||
| // use the elastic-single-index backend and configure index base names. | ||
| envVars = append(envVars, corev1.EnvVar{Name: "BACKEND", Value: "elastic-single-index"}) | ||
| for _, index := range l.cfg.Tenant.Spec.Indices { | ||
| envVars = append(envVars, index.EnvVar()) | ||
| } | ||
| } |
cluster This will be set operator bootstrap config map and enabled when migrating from multi-index format to single-index format. Linseed will reconfigure its environment variables to set the correct backend for the indices it is using and also set the base index name used for Cloud. * (logstorage-users): Provision Elasticsearch users from the operator while migrating to single-index storage Single-tenant clusters migrating to single-index storage need Linseed to hold RBAC for the new calico_* indices. es-kube-controllers cannot grant that, so during the migration the operator's log-storage users controller takes over user provisioning and es-kube-controllers stops running its elasticsearch configuration controller. - Run the users controller in single-tenant mode when IndexMigration is set, building the tenant configuration from the cloud config ConfigMap since single-tenant clusters have no Tenant resource. - Name the single-tenant Linseed and Dashboards users the way es-kube-controllers named them (<name>-<tenantID>-secure), and repoint existing credential secrets at those users while keeping their passwords, so credentials provisioned before the migration keep resolving. - Declare the standard single-index names on the Tenant that CloudConfig.ToTenant builds, gated on the caller opting in, so that clusters which are not migrating keep falling back to their existing index names. Sort the declared indices, as they are generated from a map. - Report the users TigeraStatus in the log-storage conditions aggregate while migrating. * (logstorage-conditions): Stop the log-storage conditions controller from writing on every reconcile updateConditions built its result by ranging over the desiredConditions map, so the order of LogStorage.Status.Conditions was randomized on every reconcile. Conditions is an atomic list, so a reorder is a real change to the stored object: each reconcile bumped the resourceVersion, and since this controller also watches LogStorage, that re-enqueued itself. The write loop ran continuously, and reconciles fired faster than the informer cache could converge - so reconciles read a stale tigera-secure and their status updates were rejected with "the object has been modified". Sort the conditions by type so the stored list is stable - an unchanged reconcile then computes a list identical to the stored one, which the API server discards without bumping the resourceVersion - and requeue instead of erroring when an update does hit a conflict. * (fix): Use CalicoEnterprise instead of TigeraEnterprise * (logstorage-users): Provision Elasticsearch users for all single tenant clusters Calico Cloud single-tenant clusters had their Elasticsearch users provisioned by es-kube-controllers, and the operator only took over while migrating to single-index storage. Take over for all of them, and grant Linseed access to the indices its cluster actually stores data in rather than to both name formats. - Run the log-storage users controller for every Calico Cloud single-tenant cluster, and stop es-kube-controllers running its elasticsearch configuration controller there so that it does not overwrite the users we own. Wait for the cloud config ConfigMap rather than erroring when it is not there yet. - Derive the Linseed role's index privileges from the tenant: the declared base index names when the cluster stores data in single-index format, the multi-index names otherwise - dropping the tenant qualifier for clusters on their own Elasticsearch, whose indices do not carry it. - Report the users TigeraStatus in the log-storage conditions aggregate for Calico Cloud rather than only while migrating. - Rename the index migration knob to USE_SINGLE_INDEX / UseSingleIndex, matching the naming already used by the linseed controller and render code. - Move CloudStandardIndices out of the API module and unexport it, as it is only consumed when building the single-tenant Tenant from the cloud config. * (logstorage-users): Address review feedback on Linseed index privileges - Skip indices with an empty base index name when building the Linseed role's index privileges. A Tenant declaring an index without a base name - whether misconfigured, or carrying a DataType added later without a mapping - would otherwise be wildcarded into "*", granting Linseed access to every index in Elasticsearch. Fall back to Linseed's default calico_ names when no usable base index name remains, which is what a Tenant declaring no indices at all already got. - Fix a comment on the single-tenant Linseed backend, which described the branch as migrating to multi-tenant style indices when it is gated by UseSingleIndex and migrating to single-index storage.
Policy activity is only ever stored in single-index format, so it is named outside the tigera_secure_ee_ pattern even on clusters that have not moved to single-index storage. The multi-index role we provision granted only that pattern, and since the role name matches the user name, putting it replaced the definition es-kube-controllers wrote - which did include the index. Linseed was then denied indices:admin/aliases/get when ingesting policy activity logs. Grant calico_policy_activity* alongside the multi-index pattern. The wildcard covers both Linseed's default index name and the calico_policy_activity_standard name pinned for clusters sharing an external Elasticsearch.
es-kube-controllers' authorization controller sweeps Elasticsearch on every resync and deletes any user that is neither marked with the "system:serviceaccount" full_name nor present in its OIDC user cache. The operator's User struct had no full_name field at all, so the Linseed and Dashboards installer users it provisions were created with an empty one and swept within a resync period, then recreated on the next reconcile - leaving the user flapping while both components are enabled. Add FullName to User, set it on both users, and send full_name in the create request. It is only sent when non-empty: the request replaces the user document, so sending it empty would strip the marker off a user that es-kube-controllers had already created with it.
0cd8b0f to
7c392f5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (2)
pkg/enterprise/utils/tenant_test.go:25
- This Ginkgo spec file is in package
utils, but the existing suite runner in this directory isutils_suite_test.goin packageutils_test. As a result, these specs won’t be executed bygo test(noTest*function callsRunSpecsin theutilspackage). Add a small suite entry point in this package (or move this file toutils_test).
package utils
import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
pkg/enterprise/utils/tenant.go:45
WithStandardIndicesappends totenant.Spec.Indiceswithout checking what’s already there, so applying this option more than once (or combining it with other options that also add indices) will duplicate entries and can reorder them unexpectedly. Making the option idempotent by skipping already-present data types avoids that footgun.
func WithStandardIndices() TenantOption {
return func(tenant *v1.Tenant) {
for dataType := range v1.DataTypes {
tenant.Spec.Indices = append(tenant.Spec.Indices, v1.Index{DataType: dataType, BaseIndexName: cloudStandardIndices[dataType]})
}
WithStandardIndices appended an entry for every data type without checking what the Tenant already declared, so applying it more than once - or alongside an option that declares its own indices - duplicated entries. Skip data types that are already present, letting an explicitly declared base index name win over the standard one rather than being duplicated or overridden. No current caller hits this: TenantFromCloudConfig always starts from a tenant with no indices, and every call site passes at most one option. This is hardening of a shared helper, not a behaviour change for any caller. Cover both cases - applying the option twice, and applying it after an option that already declared an index. Both specs fail against the previous implementation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (1)
pkg/controller/logstorage/users/users_controller.go:288
- GetClusterID() failures are currently all treated as ResourceReadError with a "Waiting..." message and the reconcile returns the error. For a not-found cluster-info ConfigMap this will spam errors/backoff rather than treating it as a not-ready dependency. Consider handling IsNotFound separately and requeue without returning an error; keep returning the error for other failures.
if r.multiTenant {
clusterID, err := eutils.GetClusterID(ctx, r.client)
if err != nil {
r.status.SetDegraded(operatorv1.ResourceReadError, "Waiting for the cluster ID to be available", err, reqLogger)
return reconcile.Result{}, err
}
Description
indices it is using and also set the base index name used for Cloud.
Release Note
For PR author
make gen-filesmake gen-versionsFor PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bugif this is a bugfix.kind/enhancementif this is a a new feature.enterpriseif this PR applies to Calico Enterprise only.