[Feature] [Platform] Registry Secret #1982
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for managing registry credentials secrets for license authentication in the ArangoDB Kubernetes operator. The main changes include automatically creating and updating a Kubernetes secret containing registry credentials when API-based licensing is configured, and integrating those credentials into the deployment profiles.
Key changes:
- Added
RegistryConfigmethod to the license manager client that generates registry authentication configuration - Introduced
PatchSecretDatahelper function for patching Kubernetes secret data - Added
InputHashfield to license status to track changes to license credentials - Integrated registry credentials secret into ArangoDB profiles for image pulling
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/util/k8sutil/patcher/secret.go | New patcher function for updating secret data |
| pkg/util/k8sutil/license.go | Added Hash method for license credentials |
| pkg/util/constants/gateway.go | Extracted ChecksumKey constant for reuse |
| pkg/license_manager/client.go | Added RegistryConfig method for generating registry auth |
| pkg/deployment/resources/arango_profiles.go | Integrated image pull secrets into profiles |
| pkg/deployment/reconcile/plan_builder_license.go | Added validation for registry secret changes |
| pkg/deployment/reconcile/action_license_generate.go | Added registry secret creation/update logic |
| pkg/deployment/reconcile/action_license_set.go | Updated to track InputHash in license status |
| pkg/deployment/pod/encryption.go | Added helper for registry secret naming |
| pkg/apis/deployment/v*/deployment_status_license.go | Added InputHash field to API definitions |
| pkg/platform/license_*.go | Refactored to use lmanager alias consistently |
| pkg/util/cli/lm.go | Updated to use lmanager alias |
| .golangci.yaml | Added lmanager alias configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cache := a.actionCtx.ACS().CurrentClusterCache() | ||
|
|
||
| if s, ok := cache.Secret().V1().GetSimple(pod.GetLicenseRegistryCredentialsSecretName(a.actionCtx.GetName())); ok { | ||
| if string(util.Optional(s.Data, utilConstants.ChecksumKey, []byte{})) != l.API.Hash() { |
There was a problem hiding this comment.
The checksum comparison is duplicated in both the if and else branches (lines 176 and 246 in plan_builder_license.go). Consider extracting this logic into a helper function to improve maintainability and ensure consistent validation logic across the codebase.
| if _, err := cache.Client().Kubernetes().CoreV1().Secrets(a.actionCtx.GetNamespace()).Create(ctx, &core.Secret{ | ||
| ObjectMeta: meta.ObjectMeta{ | ||
| Name: pod.GetLicenseRegistryCredentialsSecretName(a.actionCtx.GetName()), | ||
| OwnerReferences: []meta.OwnerReference{ | ||
| a.actionCtx.GetAPIObject().AsOwner(), | ||
| }, | ||
| }, | ||
| Data: map[string][]byte{ | ||
| core.DockerConfigJsonKey: token, | ||
| utilConstants.ChecksumKey: []byte(l.API.Hash()), | ||
| }, | ||
| Type: core.SecretTypeDockerConfigJson, | ||
| }, meta.CreateOptions{}); err != nil { |
There was a problem hiding this comment.
The Secret creation logic (lines 201-213) duplicates the data structure from the patch operation (lines 186-189). Consider extracting a helper function that constructs the Secret data map to avoid duplication and ensure consistency between create and update operations.
No description provided.