tici: move meta config to secret#6808
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
54734be to
478eaed
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces explicit TiCI meta TiDB authentication configuration via a user-provided Secret, ensuring operator-generated TiCI meta ConfigMaps remain credential-free and that TiCI meta pods roll when the referenced Secret data changes.
Changes:
- Add
spec.tici.meta.tidbAuthAPI/CRD support and validation (including forbiddingpasswordSecret.optional: true). - Generate TiCI meta runtime
tici.tomlin an EmptyDir at container startup by combining a credential-free template ConfigMap with a mounted Secret. - Watch referenced Secrets and annotate the TiCI meta PodTemplate with a hash to trigger rolling updates on Secret rotation.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/manager/member/tici_member_manager.go | Implements TiCI meta Secret-based auth, runtime config generation, and rolling-upgrade detection on auth hash annotation. |
| pkg/manager/member/tici_member_manager_test.go | Adds unit tests for the meta start script, config generation, auth hashing, and warning/requeue behavior. |
| pkg/controller/tidbcluster/tidb_cluster_controller.go | Adds Secret informer handling to enqueue TidbClusters when TiCI meta auth Secrets change. |
| pkg/controller/tidbcluster/tidb_cluster_controller_test.go | Adds coverage for Secret-triggered enqueue logic (including tombstone handling). |
| pkg/apis/pingcap/v1alpha1/zz_generated.deepcopy.go | Adds deepcopy support for the new TiCI meta auth struct. |
| pkg/apis/pingcap/v1alpha1/validation/validation.go | Adds validation for spec.tici.meta.tidbAuth and forbids passwordSecret.optional: true. |
| pkg/apis/pingcap/v1alpha1/validation/validation_test.go | Adds tests for TiCI meta TiDB auth validation cases. |
| pkg/apis/pingcap/v1alpha1/types.go | Introduces the TiCITiDBAuth API type and wires it into TiCIMetaSpec. |
| pkg/apis/pingcap/v1alpha1/openapi_generated.go | Updates OpenAPI definitions for the new TiCI meta auth fields. |
| manifests/crd/v1/pingcap.com_tidbclusters.yaml | Updates CRD schema to include spec.tici.meta.tidbAuth and its validations. |
| manifests/crd.yaml | Updates bundled CRD schema to include spec.tici.meta.tidbAuth and its validations. |
| docs/api-references/docs.md | Updates generated API reference docs for the new TiCI meta auth configuration. |
Files not reviewed (1)
- pkg/apis/pingcap/v1alpha1/zz_generated.deepcopy.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func renderTiCIMetaStartScript(tc *v1alpha1.TidbCluster, headlessSvcName string, tidbAuth *tiCIMetaTiDBAuth) string { | ||
| configTemplatePath := fmt.Sprintf("%s/tici.toml", ticiMetaConfigTemplateMountPath) | ||
| configPath := fmt.Sprintf("%s/tici.toml", ticiMetaConfigMountPath) | ||
|
|
||
| lines := []string{ | ||
| "set -e", | ||
| fmt.Sprintf("config_template=%q", configTemplatePath), | ||
| fmt.Sprintf("config_file=%q", configPath), | ||
| } | ||
| if tidbAuth != nil { | ||
| authPath := fmt.Sprintf("%s/%s", ticiMetaTiDBAuthMountPath, ticiMetaTiDBAuthFileName) | ||
| lines = append(lines, | ||
| fmt.Sprintf("tidb_auth_user=%q", tidbAuth.User), | ||
| fmt.Sprintf("tidb_auth_file=%q", authPath), |
| tcs, err := c.deps.TiDBClusterLister.TidbClusters(secret.Namespace).List(labels.Everything()) | ||
| if err != nil { | ||
| utilruntime.HandleError(fmt.Errorf("couldn't list TidbClusters for TiCI meta TiDB auth secret %s/%s: %v", secret.Namespace, secret.Name, err)) | ||
| return | ||
| } |
| // The referenced Secret key is required; optional must not be true. | ||
| // +kubebuilder:validation:Required |
| if !ok { | ||
| return false | ||
| } | ||
| if err := json.Unmarshal([]byte(lastAppliedConfig), &oldStsSpec); err != nil { |
|
@3pointer: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What problem does this PR solve?
TiCI meta needs to authenticate to TiDB without persisting TiDB credential data in the operator-generated ConfigMap/TOML. The auth source should also be explicit instead of implicitly reusing the TiDB initializer Secret.
What is changed and how does it work?
Add explicit TiCI meta TiDB auth config:
Keep the generated TiCI meta ConfigMap credential-free. When
tidbAuth.passwordSecretis configured, the controller validates the referenced Secret/key, mounts that Secret as a required volume, and generates the credential-bearingtici.tomlonly in the container's runtime EmptyDir at startup.The API/CRD validation rejects
passwordSecret.optional: truebecause the Secret volume is required once TiCI auth is explicitly configured.Add a hash of the referenced Secret key to the TiCI meta PodTemplate and watch Secret changes that are referenced by
spec.tici.meta.tidbAuth.passwordSecret, so Secret data rotation enqueues the TidbCluster and rolls TiCI meta pods. TiCI's rolling-upgrade check also compares PodTemplate annotations, so an auth-hash-only change sets the rolling partition to0.If the configured Secret does not exist, the controller emits a Warning event and requeues. If the Secret exists but the configured key is missing, the controller emits a Warning event and returns a clear error.
Code changes
Tests
Side effects
Related changes
Release Notes