-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add possibility to prefix stack namespaces #341
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,7 @@ func main() { | |
| env string | ||
| licenceSecret string | ||
| utilsVersion string | ||
| namespacePrefix string | ||
| ) | ||
| flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") | ||
| flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") | ||
|
|
@@ -77,6 +78,7 @@ func main() { | |
| flag.StringVar(&env, "env", "staging", "The current environment in use for the operator") | ||
| flag.StringVar(&licenceSecret, "licence-secret", "", "The licence secret that contains the token and the issuer") | ||
| flag.StringVar(&utilsVersion, "utils-version", "latest", "The version of the operator utils image") | ||
| flag.StringVar(&namespacePrefix, "namespace-prefix", "", "The prefix to use when creating namespaces (format: <prefix><organisationID>-<stackID>)") | ||
|
Member
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. Use CRD Settings instead of Flag on Operator |
||
| opts := zap.Options{ | ||
| Development: false, | ||
| } | ||
|
|
@@ -119,10 +121,11 @@ func main() { | |
| } | ||
|
|
||
| platform := core.Platform{ | ||
| Region: region, | ||
| Environment: env, | ||
| LicenceSecret: licenceSecret, | ||
| UtilsVersion: utilsVersion, | ||
| Region: region, | ||
| Environment: env, | ||
| LicenceSecret: licenceSecret, | ||
| UtilsVersion: utilsVersion, | ||
| NamespacePrefix: namespacePrefix, | ||
| } | ||
|
|
||
| if err := core.Setup(mgr, platform); err != nil { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,7 +110,7 @@ func Reconcile(ctx core.Context, stack *v1beta1.Stack, req *v1beta1.ResourceRefe | |
| oldResource := &unstructured.Unstructured{} | ||
| oldResource.SetGroupVersionKind(gvk) | ||
| err := ctx.GetClient().Get(ctx, types.NamespacedName{ | ||
| Namespace: stack.Name, | ||
| Namespace: core.GetNamespaceName(ctx.GetPlatform(), stack.Name), | ||
| Name: req.Status.SyncedResource, | ||
| }, oldResource) | ||
| if client.IgnoreNotFound(err) != nil { | ||
|
|
@@ -147,7 +147,7 @@ func Reconcile(ctx core.Context, stack *v1beta1.Stack, req *v1beta1.ResourceRefe | |
|
|
||
| newResource := &unstructured.Unstructured{} | ||
| newResource.SetGroupVersionKind(gvk) | ||
| newResource.SetNamespace(stack.Name) | ||
| newResource.SetNamespace(core.GetNamespaceName(ctx.GetPlatform(), stack.Name)) | ||
| newResource.SetName(req.Spec.Name) | ||
|
Comment on lines
+150
to
151
Contributor
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. 🧩 Analysis chainVerify all namespace usages are updated. The namespace resolution correctly uses Run the following script to find potential locations where 🏁 Script executed: #!/bin/bash
# Search for patterns where stack.Name might be used as a namespace
# Focus on SetNamespace, Namespace: stack.Name, and types.NamespacedName patterns
echo "=== Checking SetNamespace calls ==="
rg -n 'SetNamespace\(\s*stack\.Name\s*\)' --type go
echo "=== Checking Namespace field assignments ==="
rg -n 'Namespace:\s*stack\.Name' --type go
echo "=== Checking types.NamespacedName with stack.Name ==="
rg -n 'types\.NamespacedName\s*{[^}]*Namespace:\s*stack\.Name' --type go
echo "=== Checking direct stack.Name usage in namespace contexts ==="
# Look for GetClient().Get/Create/Update with stack.Name
rg -nC3 '(Get|Create|Update|Delete|List)\(ctx,.*stack\.Name' --type go | grep -v GetNamespaceNameLength of output: 1383 Length of output: 2179 🏁 Script executed: #!/bin/bash
# Check a few production code locations to understand context
echo "=== Context for ingress.go:129 ==="
rg -nB10 -A5 'Namespace: stack.Name' internal/resources/gateways/ingress.go | head -20
echo ""
echo "=== Context for authclients/init.go:54 ==="
rg -nB10 -A5 'Namespace: stack.Name' internal/resources/authclients/init.go | head -20Length of output: 1000 Update namespace usage across multiple files to use GetNamespaceName helper consistently. The reviewed file correctly uses
These must be updated to use 🤖 Prompt for AI Agents |
||
|
|
||
| _, err = controllerutil.CreateOrUpdate(ctx, ctx.GetClient(), newResource, func() error { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Add migration guidance for existing deployments.
The documentation doesn't address what happens to existing stacks when the namespace prefix is added or changed. When the prefix is enabled on an existing operator deployment, new namespaces will be created rather than renaming existing ones, potentially leaving orphaned resources in the old namespaces.
Consider adding a warning and migration instructions for existing deployments.
Additionally, the documentation states the format is
<organisationID>-<stackID>, but the code usesstackNamedirectly. Can you clarify the relationship between these concepts? IfstackNamealready includes the organization ID, the documentation should reflect this.I'll verify the review comment by examining the codebase for the namespace prefix implementation and how it handles existing deployments.
I'll verify the review comment by examining the codebase for the namespace prefix implementation and how it handles existing deployments.
🏁 Script executed:
Length of output: 21345
🏁 Script executed:
Length of output: 21345
🏁 Script executed:
Length of output: 1508
Clarify namespace format documentation and add migration guidance for prefix changes.
The codebase confirms both concerns:
Migration guidance is missing: When the namespace prefix is added or changed on an existing deployment, the operator will create new namespaces for stacks (e.g.,
dev-myorg-mystack) while leaving the original namespaces (e.g.,myorg-mystack) orphaned and unmanaged. There is no cleanup or migration logic to handle this scenario.Documentation format is misleading: The documentation states the format is
<organisationID>-<stackID>as separate components, but the code implementation (ininternal/core/names.go) treatsstackNameas an atomic value that already contains the full identifier. The format description should clarify thatstackNameis already in the formorganisation-stack.Update README to:
<prefix><stackName>where stackName includes both organization and stack identifiers🤖 Prompt for AI Agents