Skip to content

feat(cli,sdk,manifests): credential name resolution + platform ingress NetworkPolicy #1566

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
markturansky wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat(cli,sdk,manifests): credential name resolution + platform ingress NetworkPolicy #1566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f416ffa
fix(manifests): replace broken NetworkPolicy with proper platform ing…
May 12, 2026
d3e8af6
Merge branch 'main' into fix/platform-ingress-netpol-base
mergify[bot] May 14, 2026
9f7a581
Merge branch 'main' into fix/platform-ingress-netpol-base
mergify[bot] May 14, 2026
744567d
Merge branch 'main' into fix/platform-ingress-netpol-base
mergify[bot] May 14, 2026
a5ef660
feat(cli,sdk): credential name resolution + FindByName across all sub…
May 18, 2026
23d8958
fix(runner,cp): credential token fetch uses correct path + project-sc…
May 19, 2026
c02c788
fix(runner): update test for global credential path + make coderabbit…
May 19, 2026
023434e
fix(manifests): restore permissive NetworkPolicy for Kind E2E overlay
May 19, 2026
67b00e6
fix(manifests): Kind overlay patches for api-server, control-plane, a…
May 20, 2026
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ scripts/feedback-loop/.last-run
**/minio-credentials-secret.yaml
**/postgresql-credentials-secret.yaml
**/unleash-credentials-secret.yaml
.secrets/

# One-off analysis and experiments
repomix-analysis/
Expand Down
4 changes: 2 additions & 2 deletions components/ambient-cli/cmd/acpctl/apply/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func applyProject(ctx context.Context, client *sdkclient.Client, doc resource) (
}

func applyCredential(ctx context.Context, client *sdkclient.Client, doc resource) (applyResult, error) {
existing, err := client.Credentials().Get(ctx, doc.Name)
existing, err := client.Credentials().FindByName(ctx, doc.Name)
if err != nil {
token := os.ExpandEnv(doc.Token)
Comment on lines +246 to 248
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t treat every lookup error as create-miss.

Line 247 routes to create on any FindByName error. This should only happen on not-found; otherwise transient/API failures are misclassified and can produce incorrect create behavior.

As per coding guidelines, “Handle errors and edge cases explicitly rather than ignoring them.”

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/ambient-cli/cmd/acpctl/apply/cmd.go` around lines 246 - 248, The
code treats any error from client.Credentials().FindByName(ctx, doc.Name) as a
"not found" and falls through to create; change this so only a genuine not-found
error triggers creation and all other errors are returned/handled. Specifically,
in the block around existing, err := client.Credentials().FindByName(ctx,
doc.Name) (and the subsequent token := os.ExpandEnv(doc.Token) / create path),
detect the provider/API "not found" sentinel (or inspect the returned error for
a NotFound/IsNotFound condition) and only execute the create logic when that
condition is true; for any other err, propagate or log and return the error
instead of proceeding to create. Ensure you reference
client.Credentials().FindByName, doc.Name and the create branch when making the
conditional change.

builder := sdktypes.NewCredentialBuilder().
Expand Down Expand Up @@ -271,7 +271,7 @@ func applyCredential(ctx context.Context, client *sdkclient.Client, doc resource
if buildErr != nil {
return applyResult{}, buildErr
}
if _, createErr := client.Credentials().Create(ctx, cred); createErr != nil {
if _, createErr := client.Credentials().CreateCompat(ctx, cred); createErr != nil {
return applyResult{}, createErr
}
return applyResult{Kind: "Credential", Name: doc.Name, Status: "created"}, nil
Expand Down
39 changes: 34 additions & 5 deletions components/ambient-cli/cmd/acpctl/credential/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/ambient-code/platform/components/ambient-cli/pkg/config"
"github.com/ambient-code/platform/components/ambient-cli/pkg/connection"
"github.com/ambient-code/platform/components/ambient-cli/pkg/output"
sdkclient "github.com/ambient-code/platform/components/ambient-sdk/go-sdk/client"
sdktypes "github.com/ambient-code/platform/components/ambient-sdk/go-sdk/types"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -103,7 +104,11 @@ var getCmd = &cobra.Command{
ctx, cancel := context.WithTimeout(context.Background(), cfg.GetRequestTimeout())
defer cancel()

credential, err := client.Credentials().Get(ctx, args[0])
credID, err := resolveCredentialID(ctx, client, args[0])
if err != nil {
return err
}
credential, err := client.Credentials().Get(ctx, credID)
if err != nil {
return fmt.Errorf("get credential %q: %w", args[0], err)
}
Expand Down Expand Up @@ -187,7 +192,7 @@ var createCmd = &cobra.Command{
return fmt.Errorf("build credential: %w", err)
}

created, err := client.Credentials().Create(ctx, cred)
created, err := client.Credentials().CreateCompat(ctx, cred)
if err != nil {
return fmt.Errorf("create credential: %w", err)
}
Expand Down Expand Up @@ -259,7 +264,11 @@ var updateCmd = &cobra.Command{
patch = patch.Annotations(updateArgs.annotations)
}

updated, err := client.Credentials().Update(ctx, args[0], patch.Build())
credID, err := resolveCredentialID(ctx, client, args[0])
if err != nil {
return err
}
updated, err := client.Credentials().Update(ctx, credID, patch.Build())
if err != nil {
return fmt.Errorf("update credential: %w", err)
}
Expand Down Expand Up @@ -296,7 +305,11 @@ var deleteCmd = &cobra.Command{
ctx, cancel := context.WithTimeout(context.Background(), cfg.GetRequestTimeout())
defer cancel()

if err := client.Credentials().Delete(ctx, args[0]); err != nil {
credID, err := resolveCredentialID(ctx, client, args[0])
if err != nil {
return err
}
if err := client.Credentials().Delete(ctx, credID); err != nil {
return fmt.Errorf("delete credential: %w", err)
}

Expand Down Expand Up @@ -329,7 +342,11 @@ var tokenCmd = &cobra.Command{
ctx, cancel := context.WithTimeout(context.Background(), cfg.GetRequestTimeout())
defer cancel()

resp, err := client.Credentials().GetToken(ctx, args[0])
credID, err := resolveCredentialID(ctx, client, args[0])
if err != nil {
return err
}
resp, err := client.Credentials().GetToken(ctx, credID)
if err != nil {
return fmt.Errorf("get token for credential %q: %w", args[0], err)
}
Expand Down Expand Up @@ -451,6 +468,18 @@ func init() {
bindCmd.Flags().StringVar(&bindArgs.project, "project", "", "Project to bind the credential to (required)")
}

func resolveCredentialID(ctx context.Context, client *sdkclient.Client, nameOrID string) (string, error) {
cred, err := client.Credentials().Get(ctx, nameOrID)
if err == nil {
return cred.ID, nil
}
cred, err = client.Credentials().FindByName(ctx, nameOrID)
if err != nil {
return "", fmt.Errorf("credential %q not found by ID or name", nameOrID)
}
Comment on lines +471 to +479
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Only fallback to name lookup on NotFound errors.

Line 472 currently falls back to FindByName for any Get failure, which can mask transport/auth/server errors as “not found.” Gate fallback strictly to 404/not-found and propagate other errors with context.

As per coding guidelines, “Handle errors and edge cases explicitly rather than ignoring them.”

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/ambient-cli/cmd/acpctl/credential/cmd.go` around lines 471 - 479,
The resolveCredentialID function currently falls back to
client.Credentials().FindByName on any error from client.Credentials().Get,
which can hide transport/auth/server errors; change it so that after calling
client.Credentials().Get(ctx, nameOrID) you only call FindByName when the Get
error indicates a not-found/404 condition (use the SDK's NotFound sentinel or
inspect the HTTP/status code), and for any other error return that error wrapped
with context (e.g., "failed to get credential %q") instead of falling back; keep
the final FindByName call and its existing not-found error message unchanged.

return cred.ID, nil
}

func printCredentialTable(printer *output.Printer, credentials []sdktypes.Credential) error {
columns := []output.Column{
{Name: "ID", Width: 27},
Expand Down
6 changes: 5 additions & 1 deletion components/ambient-cli/cmd/acpctl/delete/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,11 @@ func run(cmd *cobra.Command, cmdArgs []string) error {
return nil

case "credential", "credentials", "cred", "creds":
if err := client.Credentials().Delete(ctx, name); err != nil {
deleteID := name
if cred, findErr := client.Credentials().FindByName(ctx, name); findErr == nil {
deleteID = cred.ID
}
if err := client.Credentials().Delete(ctx, deleteID); err != nil {
Comment on lines +136 to +139
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid silent fallback when name resolution fails.

Line 136 ignores FindByName errors and proceeds with Delete anyway. Restrict fallback to explicit not-found and return other errors; otherwise API/auth failures are hidden.

As per coding guidelines, “Handle errors and edge cases explicitly rather than ignoring them.”

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/ambient-cli/cmd/acpctl/delete/cmd.go` around lines 136 - 139, The
code currently ignores all errors from client.Credentials().FindByName and
proceeds to call Delete using deleteID; change this so that if FindByName
returns an error you only continue when the error explicitly means "not found"
and otherwise return/propagate the error. Concretely, after calling
client.Credentials().FindByName(ctx, name) check if findErr != nil and then if
!errors.Is(findErr, <package>.ErrNotFound) (or use the repository/client
IsNotFound helper if available) return or wrap findErr; only when the error is
the NotFound sentinel keep using deleteID (or set deleteID from cred when found)
before calling client.Credentials().Delete(ctx, deleteID). Ensure you import
errors and use the package-specific ErrNotFound/IsNotFound symbol to distinguish
not-found from other failures.

return fmt.Errorf("delete credential %q: %w", name, err)
}
fmt.Fprintf(cmd.OutOrStdout(), "credential/%s deleted\n", name)
Expand Down
5 changes: 4 additions & 1 deletion components/ambient-cli/cmd/acpctl/describe/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ func run(cmd *cobra.Command, cmdArgs []string) error {
case "credential", "credentials", "cred", "creds":
cred, err := client.Credentials().Get(ctx, name)
if err != nil {
return fmt.Errorf("describe credential %q: %w", name, err)
cred, err = client.Credentials().FindByName(ctx, name)
if err != nil {
return fmt.Errorf("describe credential %q: %w", name, err)
}
}
return printer.PrintJSON(cred)

Expand Down
5 changes: 4 additions & 1 deletion components/ambient-cli/cmd/acpctl/get/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,10 @@ func getCredentials(ctx context.Context, client *sdkclient.Client, printer *outp
if name != "" {
cred, err := client.Credentials().Get(ctx, name)
if err != nil {
return fmt.Errorf("get credential %q: %w", name, err)
cred, err = client.Credentials().FindByName(ctx, name)
if err != nil {
return fmt.Errorf("get credential %q: %w", name, err)
}
Comment on lines 473 to +478
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use not-found-aware fallback in get credentials.

Line 474 should not automatically switch to FindByName for every Get error. Keep fallback for not-found only, and return non-404 failures directly.

As per coding guidelines, “Handle errors and edge cases explicitly rather than ignoring them.”

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/ambient-cli/cmd/acpctl/get/cmd.go` around lines 473 - 478, The
current logic calls client.Credentials().FindByName whenever
client.Credentials().Get returns any error; change this so only a not-found
error triggers the fallback. In the block around client.Credentials().Get(ctx,
name) and client.Credentials().FindByName(ctx, name), detect a not-found
condition (e.g., errors.Is(err, <notFoundSentinel>) or checking gRPC
status.Code(err) == codes.NotFound) and only then call FindByName; for any other
error return fmt.Errorf("get credential %q: %w", name, err) immediately. Keep
the existing error wrap/message and the cred variable usage unchanged.

}
if printer.Format() == output.FormatJSON {
return printer.PrintJSON(cred)
Expand Down
Loading
Loading