Add secretRef and configMapRef support on ServiceContext#3232
Add secretRef and configMapRef support on ServiceContext#3232michaeljguarino merged 4 commits intomasterfrom
Conversation
Greptile SummaryAdds
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| go/controller/api/v1alpha1/servicecontext_types.go | Adds ConfigMapRef (ObjectReference) and SecretRef (SecretReference) fields to ServiceContextSpec. Clean type additions with proper kubebuilder tags. |
| go/controller/internal/controller/servicecontext_controller.go | Core logic for merging ConfigMap/Secret data into service context configuration. Has inconsistent namespace fallback between ConfigMap (uses SC namespace) and Secret (uses "default" via utils.GetSecret). New OnConfigMapChange handler added. |
| go/controller/internal/controller/servicecontext_controller_test.go | Good test covering merged configuration from ConfigMap, Secret, and existing Configuration. Verifies owner annotations are set correctly. |
| go/controller/config/crd/bases/deployments.plural.sh_servicecontexts.yaml | CRD schema updated with configMapRef and secretRef properties. Generated from types. |
| go/controller/docs/api.md | API documentation updated with new configMapRef and secretRef fields. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[ServiceContext Reconcile] --> B{handleExisting & DriftDetect?}
B -->|Skip sync| C[Mark ReadOnly & Requeue]
B -->|Proceed| D[sync]
D --> E[Parse spec.configuration JSON]
E --> F{configMapRef set?}
F -->|Yes| G[Fetch ConfigMap]
G --> H[AddOwnerRefAnnotation to ConfigMap]
H --> I[Merge ConfigMap data into config]
F -->|No| J{secretRef set?}
I --> J
J -->|Yes| K[Fetch Secret]
K --> L[AddOwnerRefAnnotation to Secret]
L --> M[Merge Secret data into config]
J -->|No| N[Marshal merged JSON]
M --> N
N --> O[SaveServiceContext to Console API]
P[Secret Changes] --> Q[OnSecretChange Watch]
Q --> R[GetOwnerRefsAnnotationRequests]
R --> A
S[ConfigMap Changes] --> T[OnConfigMapChange Watch]
T --> R
Last reviewed commit: 312612f
| if sc.Spec.SecretRef != nil { | ||
| secret, err := utils.GetSecret(ctx, r.Client, sc.Spec.SecretRef) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get secret %s/%s: %w", sc.Spec.SecretRef.Namespace, sc.Spec.SecretRef.Name, err) |
There was a problem hiding this comment.
Inconsistent namespace fallback for SecretRef
When SecretRef.Namespace is empty, utils.GetSecret falls back to "default" (see go/controller/internal/utils/kubernetes.go:193), whereas the ConfigMapRef handling above (line 128-129) falls back to sc.GetNamespace() (the ServiceContext's own namespace). This inconsistency means that if a user omits the namespace on both refs, the ConfigMap is looked up in the ServiceContext's namespace but the Secret is looked up in "default".
The ConfigMap behavior (falling back to the owner's namespace) is the more intuitive and correct approach. Consider either adding a namespace fallback before calling GetSecret, or updating GetSecret to accept the fallback namespace:
| if sc.Spec.SecretRef != nil { | |
| secret, err := utils.GetSecret(ctx, r.Client, sc.Spec.SecretRef) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to get secret %s/%s: %w", sc.Spec.SecretRef.Namespace, sc.Spec.SecretRef.Name, err) | |
| if sc.Spec.SecretRef != nil { | |
| ref := sc.Spec.SecretRef | |
| if ref.Namespace == "" { | |
| ref = &corev1.SecretReference{Name: ref.Name, Namespace: sc.GetNamespace()} | |
| } | |
| secret, err := utils.GetSecret(ctx, r.Client, ref) |
312612f to
f4ba9d7
Compare
This should give us the ability to take config maps and secrets and publish them to service context. Useful for combining with things like ASO, and crossplane.
f4ba9d7 to
8e64174
Compare
c4b98d4 to
64a9a50
Compare
38745c1 to
914e78e
Compare
95447e4 to
8f6fffd
Compare
This should give us the ability to take config maps and secrets and publish them to service context. Useful for combining with things like ASO, and crossplane.
Test Plan
unit test
Checklist
Plural Flow: console