Skip to content

fix(stack): add missing stack metadata update flag #83

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(stack): add missing stack metadata update flag #83

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions cmd/stack/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import (
)

const (
nameFlag = "name"
nameFlag = "name"
metadataFlag = "metadata"
)

type StackUpdateStore struct {
Expand Down Expand Up @@ -46,6 +47,7 @@ func NewUpdateCommand() *cobra.Command {
}),
fctl.WithBoolFlag(unprotectFlag, false, "Unprotect stacks (no confirmation on write commands)"),
fctl.WithStringFlag(nameFlag, "", "Name of the stack"),
fctl.WithStringSliceFlag(metadataFlag, []string{""}, "Metadata to use"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Bug: default []string{""} makes ParseMetadata fail when flag is omitted.

With the current default, GetStringSlice returns a single empty string, and ParseMetadata errors with “malformed metadata: ”. Use an empty slice as default.

Apply this diff:

-        fctl.WithStringSliceFlag(metadataFlag, []string{""}, "Metadata to use"),
+        fctl.WithStringSliceFlag(metadataFlag, []string{}, "Metadata key=value. Repeat the flag to add multiple entries"),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fctl.WithStringSliceFlag(metadataFlag, []string{""}, "Metadata to use"),
fctl.WithStringSliceFlag(metadataFlag, []string{}, "Metadata key=value. Repeat the flag to add multiple entries"),
🤖 Prompt for AI Agents 
In cmd/stack/update.go around line 50, the flag default is set to []string{""}
which causes GetStringSlice to return a single empty string and ParseMetadata to
fail; change the flag default to an empty slice (i.e., []string{}) so omitted
flag yields no entries, then run tests/CLI to verify ParseMetadata no longer
errors when the flag is omitted.

fctl.WithController(NewStackUpdateController()),
)
}
Expand All @@ -57,10 +59,23 @@ func (c *StackUpdateController) Run(cmd *cobra.Command, args []string) (fctl.Ren
store := fctl.GetOrganizationStore(cmd)
c.profile = store.Config.GetProfile(fctl.GetCurrentProfileName(cmd, store.Config))

// Parse user-provided metadata
userMetadata, err := fctl.ParseMetadata(fctl.GetStringSlice(cmd, metadataFlag))
if err != nil {
return nil, errors.Wrap(err, "parsing metadata")
}

// Merge user metadata with protected metadata
protected := !fctl.GetBool(cmd, unprotectFlag)
metadata := map[string]string{
fctl.ProtectedStackMetadata: fctl.BoolPointerToString(&protected),
metadata := make(map[string]string)

// Add user metadata first
for k, v := range userMetadata {
metadata[k] = v
}

// Add/override with protected metadata
metadata[fctl.ProtectedStackMetadata] = fctl.BoolPointerToString(&protected)

Comment on lines +68 to 79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

❓ Verification inconclusive

Risk: updating name with no --metadata will wipe existing metadata keys.

You always send a fresh map containing only user pairs plus the protected key. If the stack already has other metadata, those keys are lost on update. Preserve existing metadata and still enforce the protected key.

Apply this minimal merge and gate the request payload:

-    // Merge user metadata with protected metadata
-    protected := !fctl.GetBool(cmd, unprotectFlag)
-    metadata := make(map[string]string)
-    
-    // Add user metadata first
-    for k, v := range userMetadata {
-        metadata[k] = v
-    }
-    
-    // Add/override with protected metadata
-    metadata[fctl.ProtectedStackMetadata] = fctl.BoolPointerToString(&protected)
+    // Prepare metadata (user-supplied now, merge existing later)
+    protected := !fctl.GetBool(cmd, unprotectFlag)
+    metadata := make(map[string]string, len(userMetadata)+1)
+    for k, v := range userMetadata {
+        metadata[k] = v
+    }
+    // Enforce protected flag; prefer direct bool->string if available.
+    metadata[fctl.ProtectedStackMetadata] = fctl.BoolToString(protected)
@@
-    name := fctl.GetString(cmd, nameFlag)
+    // Preserve existing metadata keys not provided by the user
+    if stack.Data != nil && stack.Data.Metadata != nil {
+        for k, v := range stack.Data.Metadata {
+            if _, exists := metadata[k]; !exists {
+                metadata[k] = v
+            }
+        }
+    }
+    name := fctl.GetString(cmd, nameFlag)
@@
-    req := membershipclient.UpdateStackRequest{
-        Name:     name,
-        Metadata: pointer.For(metadata),
-    }
+    // If neither --metadata nor --unprotect was provided, avoid sending Metadata to prevent unintended changes.
+    var metaPtr *map[string]string
+    if len(userMetadata) > 0 || cmd.Flags().Changed(unprotectFlag) {
+        metaPtr = pointer.For(metadata)
+    }
+    req := membershipclient.UpdateStackRequest{
+        Name:     name,
+        Metadata: metaPtr,
+    }

If BoolToString isn’t available in your pkg, keep BoolPointerToString(&protected).

Don't overwrite existing stack metadata — merge current metadata and only send Metadata when the user changed it.

File: cmd/stack/update.go Lines: 68-79

The code builds a fresh metadata map (userMetadata + protected key) and will erase any existing metadata on update; merge stack.Data.Metadata (preserving keys not provided by the user) and only include Metadata in UpdateStackRequest when userMetadata is non-empty or the unprotect flag was changed.

-    // Merge user metadata with protected metadata
-    protected := !fctl.GetBool(cmd, unprotectFlag)
-    metadata := make(map[string]string)
-    
-    // Add user metadata first
-    for k, v := range userMetadata {
-        metadata[k] = v
-    }
-    
-    // Add/override with protected metadata
-    metadata[fctl.ProtectedStackMetadata] = fctl.BoolPointerToString(&protected)
+    // Prepare metadata (user-supplied now, merge existing later)
+    protected := !fctl.GetBool(cmd, unprotectFlag)
+    metadata := make(map[string]string, len(userMetadata)+1)
+    for k, v := range userMetadata {
+        metadata[k] = v
+    }
+    // Enforce protected flag; prefer direct bool->string if available.
+    metadata[fctl.ProtectedStackMetadata] = fctl.BoolToString(protected)
@@
-    name := fctl.GetString(cmd, nameFlag)
+    // Preserve existing metadata keys not provided by the user
+    if stack.Data != nil && stack.Data.Metadata != nil {
+        for k, v := range stack.Data.Metadata {
+            if _, exists := metadata[k]; !exists {
+                metadata[k] = v
+            }
+        }
+    }
+    name := fctl.GetString(cmd, nameFlag)
@@
-    req := membershipclient.UpdateStackRequest{
-        Name:     name,
-        Metadata: pointer.For(metadata),
-    }
+    // If neither --metadata nor --unprotect was provided, avoid sending Metadata to prevent unintended changes.
+    var metaPtr *map[string]string
+    if len(userMetadata) > 0 || cmd.Flags().Changed(unprotectFlag) {
+        metaPtr = pointer.For(metadata)
+    }
+    req := membershipclient.UpdateStackRequest{
+        Name:     name,
+        Metadata: metaPtr,
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Merge user metadata with protected metadata
protected := !fctl.GetBool(cmd, unprotectFlag)
metadata := map[string]string{
fctl.ProtectedStackMetadata: fctl.BoolPointerToString(&protected),
metadata := make(map[string]string)
// Add user metadata first
for k, v := range userMetadata {
metadata[k] = v
}
// Add/override with protected metadata
metadata[fctl.ProtectedStackMetadata] = fctl.BoolPointerToString(&protected)
// Prepare metadata (user-supplied now, merge existing later)
protected := !fctl.GetBool(cmd, unprotectFlag)
metadata := make(map[string]string, len(userMetadata)+1)
for k, v := range userMetadata {
metadata[k] = v
}
// Enforce protected flag; prefer direct bool->string if available.
metadata[fctl.ProtectedStackMetadata] = fctl.BoolToString(protected)
// Preserve existing metadata keys not provided by the user
if stack.Data != nil && stack.Data.Metadata != nil {
for k, v := range stack.Data.Metadata {
if _, exists := metadata[k]; !exists {
metadata[k] = v
}
}
}
name := fctl.GetString(cmd, nameFlag)
// If neither --metadata nor --unprotect was provided, avoid sending Metadata to prevent unintended changes.
var metaPtr *map[string]string
if len(userMetadata) > 0 || cmd.Flags().Changed(unprotectFlag) {
metaPtr = pointer.For(metadata)
}
req := membershipclient.UpdateStackRequest{
Name: name,
Metadata: metaPtr,
}
🧰 Tools
🪛 GitHub Actions: Default

[error] 68-79: CI cleanliness check failed due to uncommitted changes. The script ran 'git status --porcelain' and detected changes in cmd/stack/update.go, causing the step to exit with code 1.

🤖 Prompt for AI Agents 
In cmd/stack/update.go around lines 68-79, the current code creates a fresh
metadata map which wipes existing stack metadata; instead, fetch and clone the
current stack.Data.Metadata into your working metadata map first, then overlay
userMetadata and the protected key so keys not provided by the user are
preserved. Only set the UpdateStackRequest.Metadata field if the user provided
any metadata changes (userMetadata is non-empty) or if the protected/unprotect
flag changed compared to the original stack.Data.Metadata value (compare
original protected value vs intended protected); otherwise omit Metadata from
the request to avoid erasing existing values.

stack, res, err := store.Client().GetStack(cmd.Context(), store.OrganizationId(), args[0]).Execute()
if err != nil {
Expand Down
Loading