diff --git a/cli/cmd/definition.go b/cli/cmd/definition.go index 2bb041c..d909773 100644 --- a/cli/cmd/definition.go +++ b/cli/cmd/definition.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "net/url" "os" "path/filepath" "strconv" @@ -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, @@ -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 != "" { @@ -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: ¤t.DeployOrder, + DefaultValues: current.DefaultValues, } if chartPath != "" { @@ -443,6 +465,9 @@ Examples: if sourceRepoURL != "" { req.SourceRepoURL = sourceRepoURL } + if repositoryURL != "" { + req.RepositoryURL = repositoryURL + } if deployOrder >= 0 { req.DeployOrder = &deployOrder } @@ -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") diff --git a/cli/cmd/definition_test.go b/cli/cmd/definition_test.go index cc701d7..9f21b95 100644 --- a/cli/cmd/definition_test.go +++ b/cli/cmd/definition_test.go @@ -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", } } @@ -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) @@ -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", "") }) @@ -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", "") }) @@ -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", "") }) @@ -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") @@ -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", "") }) @@ -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", "") }) @@ -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", "") }) diff --git a/cli/pkg/types/types.go b/cli/pkg/types/types.go index c1eead5..77f8d87 100644 --- a/cli/pkg/types/types.go +++ b/cli/pkg/types/types.go @@ -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"` } // OrphanedNamespace represents a Kubernetes namespace with no matching stack record.