Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 32 additions & 6 deletions cli/cmd/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"errors"
"fmt"
"net/url"
"os"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -386,6 +387,7 @@ Examples:
stackctl definition update-chart 1 5 --chart-path /charts/kvk-core
stackctl definition update-chart 1 5 --deploy-order 6
stackctl definition update-chart 1 5 --source-repo-url https://dev.azure.com/org/project/_git/repo
stackctl definition update-chart 1 5 --repository-url oci://acr.example.com/helm
stackctl definition update-chart 1 5 --file values.yaml`,
Args: cobra.ExactArgs(2),
SilenceUsage: true,
Expand All @@ -402,11 +404,24 @@ Examples:
chartPath, _ := cmd.Flags().GetString("chart-path")
chartVersion, _ := cmd.Flags().GetString("chart-version")
sourceRepoURL, _ := cmd.Flags().GetString("source-repo-url")
repositoryURL, _ := cmd.Flags().GetString("repository-url")
deployOrder, _ := cmd.Flags().GetInt("deploy-order")
valuesFile, _ := cmd.Flags().GetString("file")

if chartPath == "" && chartVersion == "" && sourceRepoURL == "" && deployOrder < 0 && valuesFile == "" {
return fmt.Errorf("at least one of --chart-path, --chart-version, --source-repo-url, --deploy-order, or --file must be specified")
if chartPath == "" && chartVersion == "" && sourceRepoURL == "" && repositoryURL == "" && deployOrder < 0 && valuesFile == "" {
return fmt.Errorf("at least one of --chart-path, --chart-version, --source-repo-url, --repository-url, --deploy-order, or --file must be specified")
}

if repositoryURL != "" {
u, err := url.Parse(repositoryURL)
if err != nil || u.Scheme == "" || u.Host == "" {
return fmt.Errorf("--repository-url %q is not a valid URL", repositoryURL)
}
switch u.Scheme {
case "oci", "http", "https":
default:
return fmt.Errorf("--repository-url scheme %q is not supported (use oci, http, or https)", u.Scheme)
}
}

if valuesFile != "" {
Expand All @@ -427,11 +442,18 @@ Examples:
return fmt.Errorf("fetching current chart config: %w", err)
}

// Seed every field from the current record before PUT so backends
// that treat the payload as a full replacement do not wipe values
// the user didn't intend to change.
req := types.UpdateChartConfigRequest{
ChartName: current.ChartName,
ChartVersion: current.ChartVersion,
SourceRepoURL: current.SourceRepoURL,
DefaultValues: current.DefaultValues,
ChartName: current.ChartName,
RepositoryURL: current.RepoURL,
ChartPath: current.ChartPath,
ChartVersion: current.ChartVersion,
SourceRepoURL: current.SourceRepoURL,
BuildPipelineID: current.BuildPipelineID,
DeployOrder: &current.DeployOrder,
DefaultValues: current.DefaultValues,
}

if chartPath != "" {
Expand All @@ -443,6 +465,9 @@ Examples:
if sourceRepoURL != "" {
req.SourceRepoURL = sourceRepoURL
}
if repositoryURL != "" {
req.RepositoryURL = repositoryURL
}
if deployOrder >= 0 {
req.DeployOrder = &deployOrder
}
Expand Down Expand Up @@ -539,6 +564,7 @@ func init() {
definitionUpdateChartCmd.Flags().String("chart-path", "", "Chart path (e.g. /charts/kvk-core)")
definitionUpdateChartCmd.Flags().String("chart-version", "", "Chart version")
definitionUpdateChartCmd.Flags().String("source-repo-url", "", "Git repository URL for branch listing")
definitionUpdateChartCmd.Flags().String("repository-url", "", "Helm chart repository URL (e.g. oci://acr.example.com/helm)")
definitionUpdateChartCmd.Flags().Int("deploy-order", -1, "Deploy order (0+)")
definitionUpdateChartCmd.Flags().String("file", "", "File containing default values")

Expand Down
100 changes: 93 additions & 7 deletions cli/cmd/definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1012,13 +1012,17 @@ func TestDefinitionUpdateCmd_NoFlags(t *testing.T) {

func sampleChartConfig() types.ChartConfig {
return types.ChartConfig{
Base: types.Base{ID: "1", Version: "1"},
Name: "api",
RepoURL: "https://charts.example.com",
ChartName: "api-chart",
ChartVersion: "2.0.0",
ReleaseName: "api-release",
DefaultValues: "replicas: 1\nimage: app:latest",
Base: types.Base{ID: "1", Version: "1"},
Name: "api",
RepoURL: "https://charts.example.com",
ChartName: "api-chart",
ChartPath: "/charts/api",
ChartVersion: "2.0.0",
SourceRepoURL: "https://dev.azure.com/org/project/_git/repo",
BuildPipelineID: "pipeline-42",
DeployOrder: 8,
ReleaseName: "api-release",
DefaultValues: "replicas: 1\nimage: app:latest",
}
}

Expand All @@ -1040,6 +1044,15 @@ func TestDefinitionUpdateChartCmd_ChartVersion(t *testing.T) {
require.NoError(t, json.NewDecoder(r.Body).Decode(&body))
assert.Equal(t, "3.0.0", body.ChartVersion)
assert.Equal(t, chart.ChartName, body.ChartName)
// Fields not touched by the user flag must round-trip from the GET
// response back into the PUT body — that's the wipe-fix this PR exists for.
assert.Equal(t, chart.RepoURL, body.RepositoryURL)
assert.Equal(t, chart.ChartPath, body.ChartPath)
assert.Equal(t, chart.SourceRepoURL, body.SourceRepoURL)
assert.Equal(t, chart.BuildPipelineID, body.BuildPipelineID)
assert.Equal(t, chart.DefaultValues, body.DefaultValues)
require.NotNil(t, body.DeployOrder)
assert.Equal(t, chart.DeployOrder, *body.DeployOrder)

chart.ChartVersion = "3.0.0"
json.NewEncoder(w).Encode(chart)
Expand All @@ -1052,6 +1065,8 @@ func TestDefinitionUpdateChartCmd_ChartVersion(t *testing.T) {
t.Cleanup(func() {
definitionUpdateChartCmd.Flags().Set("chart-version", "")
definitionUpdateChartCmd.Flags().Set("chart-path", "")
definitionUpdateChartCmd.Flags().Set("source-repo-url", "")
definitionUpdateChartCmd.Flags().Set("repository-url", "")
definitionUpdateChartCmd.Flags().Set("deploy-order", "-1")
definitionUpdateChartCmd.Flags().Set("file", "")
})
Expand Down Expand Up @@ -1089,6 +1104,8 @@ func TestDefinitionUpdateChartCmd_ValuesFromFile(t *testing.T) {
t.Cleanup(func() {
definitionUpdateChartCmd.Flags().Set("chart-version", "")
definitionUpdateChartCmd.Flags().Set("chart-path", "")
definitionUpdateChartCmd.Flags().Set("source-repo-url", "")
definitionUpdateChartCmd.Flags().Set("repository-url", "")
definitionUpdateChartCmd.Flags().Set("deploy-order", "-1")
definitionUpdateChartCmd.Flags().Set("file", "")
})
Expand Down Expand Up @@ -1121,6 +1138,8 @@ func TestDefinitionUpdateChartCmd_DeployOrder(t *testing.T) {
t.Cleanup(func() {
definitionUpdateChartCmd.Flags().Set("chart-version", "")
definitionUpdateChartCmd.Flags().Set("chart-path", "")
definitionUpdateChartCmd.Flags().Set("source-repo-url", "")
definitionUpdateChartCmd.Flags().Set("repository-url", "")
definitionUpdateChartCmd.Flags().Set("deploy-order", "-1")
definitionUpdateChartCmd.Flags().Set("file", "")
})
Expand All @@ -1129,6 +1148,67 @@ func TestDefinitionUpdateChartCmd_DeployOrder(t *testing.T) {
require.NoError(t, err)
}

func TestDefinitionUpdateChartCmd_RepositoryURL(t *testing.T) {
chart := sampleChartConfig()
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
if r.Method == http.MethodGet {
json.NewEncoder(w).Encode(chart)
return
}
var body types.UpdateChartConfigRequest
require.NoError(t, json.NewDecoder(r.Body).Decode(&body))
assert.Equal(t, "oci://acr.example.com/helm", body.RepositoryURL)
// Other fields still seeded from GET.
assert.Equal(t, chart.ChartPath, body.ChartPath)
assert.Equal(t, chart.BuildPipelineID, body.BuildPipelineID)

chart.RepoURL = body.RepositoryURL
json.NewEncoder(w).Encode(chart)
}))
defer server.Close()

_ = setupStackTestCmd(t, server.URL)

definitionUpdateChartCmd.Flags().Set("repository-url", "oci://acr.example.com/helm")
t.Cleanup(func() {
definitionUpdateChartCmd.Flags().Set("chart-version", "")
definitionUpdateChartCmd.Flags().Set("chart-path", "")
definitionUpdateChartCmd.Flags().Set("source-repo-url", "")
definitionUpdateChartCmd.Flags().Set("repository-url", "")
definitionUpdateChartCmd.Flags().Set("deploy-order", "-1")
definitionUpdateChartCmd.Flags().Set("file", "")
})

err := definitionUpdateChartCmd.RunE(definitionUpdateChartCmd, []string{"5", "1"})
require.NoError(t, err)
}

func TestDefinitionUpdateChartCmd_RepositoryURL_Invalid(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
t.Fatal("API should not be called when --repository-url is invalid")
}))
defer server.Close()

_ = setupStackTestCmd(t, server.URL)

t.Cleanup(func() {
definitionUpdateChartCmd.Flags().Set("repository-url", "")
})

cases := []string{
"not-a-url",
"oci:/acr.example.com/helm",
"ftp://acr.example.com/helm",
}
for _, raw := range cases {
definitionUpdateChartCmd.Flags().Set("repository-url", raw)
err := definitionUpdateChartCmd.RunE(definitionUpdateChartCmd, []string{"5", "1"})
require.Error(t, err, raw)
assert.Contains(t, err.Error(), "--repository-url")
}
}

func TestDefinitionUpdateChartCmd_NoFlags(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
t.Fatal("API should not be called when no flags provided")
Expand All @@ -1140,6 +1220,8 @@ func TestDefinitionUpdateChartCmd_NoFlags(t *testing.T) {
t.Cleanup(func() {
definitionUpdateChartCmd.Flags().Set("chart-version", "")
definitionUpdateChartCmd.Flags().Set("chart-path", "")
definitionUpdateChartCmd.Flags().Set("source-repo-url", "")
definitionUpdateChartCmd.Flags().Set("repository-url", "")
definitionUpdateChartCmd.Flags().Set("deploy-order", "-1")
definitionUpdateChartCmd.Flags().Set("file", "")
})
Expand Down Expand Up @@ -1169,6 +1251,8 @@ func TestDefinitionUpdateChartCmd_JSONOutput(t *testing.T) {
t.Cleanup(func() {
definitionUpdateChartCmd.Flags().Set("chart-version", "")
definitionUpdateChartCmd.Flags().Set("chart-path", "")
definitionUpdateChartCmd.Flags().Set("source-repo-url", "")
definitionUpdateChartCmd.Flags().Set("repository-url", "")
definitionUpdateChartCmd.Flags().Set("deploy-order", "-1")
definitionUpdateChartCmd.Flags().Set("file", "")
})
Expand Down Expand Up @@ -1200,6 +1284,8 @@ func TestDefinitionUpdateChartCmd_QuietOutput(t *testing.T) {
t.Cleanup(func() {
definitionUpdateChartCmd.Flags().Set("chart-version", "")
definitionUpdateChartCmd.Flags().Set("chart-path", "")
definitionUpdateChartCmd.Flags().Set("source-repo-url", "")
definitionUpdateChartCmd.Flags().Set("repository-url", "")
definitionUpdateChartCmd.Flags().Set("deploy-order", "-1")
definitionUpdateChartCmd.Flags().Set("file", "")
})
Expand Down
20 changes: 14 additions & 6 deletions cli/pkg/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -629,13 +629,21 @@ type UpdateDefinitionRequest struct {
}

// UpdateChartConfigRequest is the request body for PUT /api/v1/stack-definitions/:id/charts/:chartID.
//
// Includes every field the backend's ChartConfig knows about, so a
// GET-then-PUT round-trip in update-chart doesn't silently wipe fields
// stackctl flags don't expose. All string fields use omitempty, so an
// empty value is dropped from the JSON body and the backend preserves
// its existing value; there is no way to clear a field via this request.
type UpdateChartConfigRequest struct {
ChartName string `json:"chart_name,omitempty" yaml:"chart_name,omitempty"`
ChartPath string `json:"chart_path,omitempty" yaml:"chart_path,omitempty"`
ChartVersion string `json:"chart_version,omitempty" yaml:"chart_version,omitempty"`
SourceRepoURL string `json:"source_repo_url,omitempty" yaml:"source_repo_url,omitempty"`
DeployOrder *int `json:"deploy_order,omitempty" yaml:"deploy_order,omitempty"`
DefaultValues string `json:"default_values,omitempty" yaml:"default_values,omitempty"`
ChartName string `json:"chart_name,omitempty" yaml:"chart_name,omitempty"`
RepositoryURL string `json:"repository_url,omitempty" yaml:"repository_url,omitempty"`
ChartPath string `json:"chart_path,omitempty" yaml:"chart_path,omitempty"`
ChartVersion string `json:"chart_version,omitempty" yaml:"chart_version,omitempty"`
SourceRepoURL string `json:"source_repo_url,omitempty" yaml:"source_repo_url,omitempty"`
BuildPipelineID string `json:"build_pipeline_id,omitempty" yaml:"build_pipeline_id,omitempty"`
DeployOrder *int `json:"deploy_order,omitempty" yaml:"deploy_order,omitempty"`
DefaultValues string `json:"default_values,omitempty" yaml:"default_values,omitempty"`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// OrphanedNamespace represents a Kubernetes namespace with no matching stack record.
Expand Down