From d2de4c3a506c4c92ba452e933e382149dc8d8748 Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Tue, 18 Aug 2026 17:02:04 +0200 Subject: [PATCH 01/10] fix(service-info): send only the renewal settings the operator asked to change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `vps service-info edit myvps --renew-period 12` used to send this: {"renew":{"automatic":false,"deleteAtExpiration":false,"forced":false, "manualPayment":false,"period":12}} The renewal settings are booleans bound to a shared struct carrying no `omitempty`, so every one of them was marshalled at its zero value and won the merge against the fetched resource. Changing the renewal period therefore also switched automatic renewal off — on a service that had been renewing itself for years, without a word in the output saying so. Webhosting already built its payload from `cmd.Flags().Changed`, and did not have the defect. This promotes that builder into `common`, so `vps` gets it too and the next `service-info edit` cannot reintroduce the bug by reaching for the struct. Reading `Changed` rather than the values keeps `--renew-automatic=false` working: pflag records a flag as changed whatever value it was given, so an explicit false is still sent while an absent flag stays absent. Both cases are covered by a test, and each test was checked against the failure it exists to catch. The shared mutable `ServiceInfoSpec` goes away with the last thing that read it, and the five flag registrations repeated across four commands become one call, which also settles the two spellings of the period's help text. Signed-off-by: Denis Hamon --- doc/ovhcloud_vps_service-info_edit.md | 10 +-- ...loud_webhosting_cdn_service-info_update.md | 10 +-- ...ebhosting_extra-sql_service-info_update.md | 10 +-- ..._local-seo_location_service-info_update.md | 10 +-- ...ovhcloud_webhosting_service-info_update.md | 10 +-- internal/cmd/vps.go | 6 +- internal/cmd/vps_test.go | 58 +++++++++++++ internal/cmd/webhosting.go | 24 +----- internal/services/common/common.go | 16 +--- internal/services/common/service_info.go | 82 +++++++++++++++++++ internal/services/vps/vps.go | 4 +- internal/services/webhosting/webhosting.go | 33 +------- 12 files changed, 179 insertions(+), 94 deletions(-) create mode 100644 internal/services/common/service_info.go diff --git a/doc/ovhcloud_vps_service-info_edit.md b/doc/ovhcloud_vps_service-info_edit.md index ca6431a61..0d4ed8f78 100644 --- a/doc/ovhcloud_vps_service-info_edit.md +++ b/doc/ovhcloud_vps_service-info_edit.md @@ -11,11 +11,11 @@ ovhcloud vps service-info edit [flags] ``` --editor Use a text editor to define parameters -h, --help help for edit - --renew-automatic Enable automatic renewal - --renew-delete-at-expiration Delete service at expiration - --renew-forced Force renewal - --renew-manual-payment Enable manual payment for renewal - --renew-period int Renewal period (in months) + --renew-automatic Renew the service automatically + --renew-delete-at-expiration Delete the service when it expires + --renew-forced Force the renewal + --renew-manual-payment Pay the renewal manually + --renew-period int Renewal period, in months ``` ### Options inherited from parent commands diff --git a/doc/ovhcloud_webhosting_cdn_service-info_update.md b/doc/ovhcloud_webhosting_cdn_service-info_update.md index 3090d068b..550fb8c24 100644 --- a/doc/ovhcloud_webhosting_cdn_service-info_update.md +++ b/doc/ovhcloud_webhosting_cdn_service-info_update.md @@ -12,11 +12,11 @@ ovhcloud webhosting cdn service-info update [flags] --editor Use a text editor to define parameters --from-file string File containing parameters -h, --help help for update - --renew-automatic Enable automatic renewal - --renew-delete-at-expiration Delete service at expiration - --renew-forced Force renewal - --renew-manual-payment Enable manual payment for renewal - --renew-period int Renewal period in months + --renew-automatic Renew the service automatically + --renew-delete-at-expiration Delete the service when it expires + --renew-forced Force the renewal + --renew-manual-payment Pay the renewal manually + --renew-period int Renewal period, in months ``` ### Options inherited from parent commands diff --git a/doc/ovhcloud_webhosting_extra-sql_service-info_update.md b/doc/ovhcloud_webhosting_extra-sql_service-info_update.md index a0885eabf..8b6c4192c 100644 --- a/doc/ovhcloud_webhosting_extra-sql_service-info_update.md +++ b/doc/ovhcloud_webhosting_extra-sql_service-info_update.md @@ -12,11 +12,11 @@ ovhcloud webhosting extra-sql service-info update [flags] --editor Use a text editor to define parameters --from-file string File containing parameters -h, --help help for update - --renew-automatic Enable automatic renewal - --renew-delete-at-expiration Delete service at expiration - --renew-forced Force renewal - --renew-manual-payment Enable manual payment for renewal - --renew-period int Renewal period in months + --renew-automatic Renew the service automatically + --renew-delete-at-expiration Delete the service when it expires + --renew-forced Force the renewal + --renew-manual-payment Pay the renewal manually + --renew-period int Renewal period, in months ``` ### Options inherited from parent commands diff --git a/doc/ovhcloud_webhosting_local-seo_location_service-info_update.md b/doc/ovhcloud_webhosting_local-seo_location_service-info_update.md index fc847ba79..01a04c78d 100644 --- a/doc/ovhcloud_webhosting_local-seo_location_service-info_update.md +++ b/doc/ovhcloud_webhosting_local-seo_location_service-info_update.md @@ -12,11 +12,11 @@ ovhcloud webhosting local-seo location service-info update [ --editor Use a text editor to define parameters --from-file string File containing parameters -h, --help help for update - --renew-automatic Enable automatic renewal - --renew-delete-at-expiration Delete service at expiration - --renew-forced Force renewal - --renew-manual-payment Enable manual payment for renewal - --renew-period int Renewal period in months + --renew-automatic Renew the service automatically + --renew-delete-at-expiration Delete the service when it expires + --renew-forced Force the renewal + --renew-manual-payment Pay the renewal manually + --renew-period int Renewal period, in months ``` ### Options inherited from parent commands diff --git a/doc/ovhcloud_webhosting_service-info_update.md b/doc/ovhcloud_webhosting_service-info_update.md index 6ee3bad93..821effd28 100644 --- a/doc/ovhcloud_webhosting_service-info_update.md +++ b/doc/ovhcloud_webhosting_service-info_update.md @@ -12,11 +12,11 @@ ovhcloud webhosting service-info update [flags] --editor Use a text editor to define parameters --from-file string File containing parameters -h, --help help for update - --renew-automatic Enable automatic renewal - --renew-delete-at-expiration Delete service at expiration - --renew-forced Force renewal - --renew-manual-payment Enable manual payment for renewal - --renew-period int Renewal period in months + --renew-automatic Renew the service automatically + --renew-delete-at-expiration Delete the service when it expires + --renew-forced Force the renewal + --renew-manual-payment Pay the renewal manually + --renew-period int Renewal period, in months ``` ### Options inherited from parent commands diff --git a/internal/cmd/vps.go b/internal/cmd/vps.go index acb2ea78f..f452ef0c6 100644 --- a/internal/cmd/vps.go +++ b/internal/cmd/vps.go @@ -221,11 +221,7 @@ func init() { ValidArgsFunction: completion.ServiceList("/v1/vps"), Run: vps.EditVpsServiceInfo, } - serviceInfoEditCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.Automatic, "renew-automatic", false, "Enable automatic renewal") - serviceInfoEditCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.DeleteAtExpiration, "renew-delete-at-expiration", false, "Delete service at expiration") - serviceInfoEditCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.Forced, "renew-forced", false, "Force renewal") - serviceInfoEditCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.ManualPayment, "renew-manual-payment", false, "Enable manual payment for renewal") - serviceInfoEditCmd.Flags().IntVar(&common.ServiceInfoSpec.Renew.Period, "renew-period", 0, "Renewal period (in months)") + common.AddServiceInfoRenewFlags(serviceInfoEditCmd) addInteractiveEditorFlag(serviceInfoEditCmd) serviceInfoCmd.AddCommand(serviceInfoEditCmd) diff --git a/internal/cmd/vps_test.go b/internal/cmd/vps_test.go index fef023dc5..467362437 100644 --- a/internal/cmd/vps_test.go +++ b/internal/cmd/vps_test.go @@ -6,6 +6,8 @@ package cmd_test import ( "encoding/json" + "io" + "net/http" "github.com/jarcoal/httpmock" "github.com/maxatome/go-testdeep/td" @@ -63,3 +65,59 @@ func (ms *MockSuite) TestVpsGetCmd(assert, require *td.T) { } }`)) } + +// registerVpsServiceInfos wires a service whose renewal is currently automatic, +// and captures whatever the CLI decides to write back. +func registerVpsServiceInfos(captured *map[string]any) { + httpmock.RegisterResponder("GET", "https://eu.api.ovh.com/v1/vps/fakeVps/serviceInfos", + httpmock.NewStringResponder(200, `{ + "serviceId": 1, + "domain": "fakeVps", + "renew": {"automatic": true, "deleteAtExpiration": false, "forced": false, "manualPayment": false, "period": 1} + }`), + ) + httpmock.RegisterResponder("PUT", "https://eu.api.ovh.com/v1/vps/fakeVps/serviceInfos", + func(req *http.Request) (*http.Response, error) { + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + var sent map[string]any + if err := json.Unmarshal(body, &sent); err != nil { + return nil, err + } + *captured = sent + return httpmock.NewStringResponse(200, `null`), nil + }, + ) +} + +// Editing the renewal period used to send every other renewal setting along +// with it, at its zero value: a service that renewed itself automatically for +// years stopped doing so, and nothing in the output said it had changed. +func (ms *MockSuite) TestVpsServiceInfoEditSendsOnlyWhatWasAsked(assert, require *td.T) { + var sent map[string]any + registerVpsServiceInfos(&sent) + + _, err := cmd.Execute("vps", "service-info", "edit", "fakeVps", "--renew-period", "12") + + require.CmpNoError(err) + renew, _ := sent["renew"].(map[string]any) + require.NotNil(renew, "the renewal block must be written") + assert.Cmp(renew["period"], float64(12), "the period the operator asked for") + assert.Cmp(renew["automatic"], true, "automatic renewal must survive untouched") +} + +// The flag being absent and the flag being set to false are different +// intentions, and pflag can tell them apart: an explicit false must be sent. +func (ms *MockSuite) TestVpsServiceInfoEditSendsAnExplicitFalse(assert, require *td.T) { + var sent map[string]any + registerVpsServiceInfos(&sent) + + _, err := cmd.Execute("vps", "service-info", "edit", "fakeVps", "--renew-automatic=false") + + require.CmpNoError(err) + renew, _ := sent["renew"].(map[string]any) + require.NotNil(renew) + assert.Cmp(renew["automatic"], false, "the operator asked for it, so it is sent") +} diff --git a/internal/cmd/webhosting.go b/internal/cmd/webhosting.go index b2baf62c0..7e5096d0f 100644 --- a/internal/cmd/webhosting.go +++ b/internal/cmd/webhosting.go @@ -605,11 +605,7 @@ func init() { ValidArgsFunction: completion.ServiceList("/v1/hosting/web"), Run: webhosting.UpdateExtraSqlServiceInfo, } - extraSQLServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.Automatic, "renew-automatic", false, "Enable automatic renewal") - extraSQLServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.DeleteAtExpiration, "renew-delete-at-expiration", false, "Delete service at expiration") - extraSQLServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.Forced, "renew-forced", false, "Force renewal") - extraSQLServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.ManualPayment, "renew-manual-payment", false, "Enable manual payment for renewal") - extraSQLServiceInfoUpdateCmd.Flags().IntVar(&common.ServiceInfoSpec.Renew.Period, "renew-period", 0, "Renewal period in months") + common.AddServiceInfoRenewFlags(extraSQLServiceInfoUpdateCmd) addParameterFileFlags(extraSQLServiceInfoUpdateCmd, true, nil, "", "", "", nil) addInteractiveEditorFlag(extraSQLServiceInfoUpdateCmd) extraSQLServiceInfoCmd.AddCommand(extraSQLServiceInfoUpdateCmd) @@ -1330,11 +1326,7 @@ func init() { ValidArgsFunction: completion.ServiceList("/v1/hosting/web"), Run: webhosting.UpdateCdnServiceInfo, } - cdnServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.Automatic, "renew-automatic", false, "Enable automatic renewal") - cdnServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.DeleteAtExpiration, "renew-delete-at-expiration", false, "Delete service at expiration") - cdnServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.Forced, "renew-forced", false, "Force renewal") - cdnServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.ManualPayment, "renew-manual-payment", false, "Enable manual payment for renewal") - cdnServiceInfoUpdateCmd.Flags().IntVar(&common.ServiceInfoSpec.Renew.Period, "renew-period", 0, "Renewal period in months") + common.AddServiceInfoRenewFlags(cdnServiceInfoUpdateCmd) addParameterFileFlags(cdnServiceInfoUpdateCmd, true, nil, "", "", "", nil) addInteractiveEditorFlag(cdnServiceInfoUpdateCmd) cdnServiceInfoCmd.AddCommand(cdnServiceInfoUpdateCmd) @@ -1468,11 +1460,7 @@ func init() { ValidArgsFunction: completion.ServiceList("/v1/hosting/web"), Run: webhosting.UpdateServiceInfo, } - serviceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.Automatic, "renew-automatic", false, "Enable automatic renewal") - serviceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.DeleteAtExpiration, "renew-delete-at-expiration", false, "Delete service at expiration") - serviceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.Forced, "renew-forced", false, "Force renewal") - serviceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.ManualPayment, "renew-manual-payment", false, "Enable manual payment for renewal") - serviceInfoUpdateCmd.Flags().IntVar(&common.ServiceInfoSpec.Renew.Period, "renew-period", 0, "Renewal period in months") + common.AddServiceInfoRenewFlags(serviceInfoUpdateCmd) addParameterFileFlags(serviceInfoUpdateCmd, true, nil, "", "", "", nil) addInteractiveEditorFlag(serviceInfoUpdateCmd) serviceInfoCmd.AddCommand(serviceInfoUpdateCmd) @@ -1583,11 +1571,7 @@ func init() { ValidArgsFunction: completion.ServiceList("/v1/hosting/web"), Run: webhosting.UpdateLocalSeoLocationServiceInfo, } - localSeoLocationServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.Automatic, "renew-automatic", false, "Enable automatic renewal") - localSeoLocationServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.DeleteAtExpiration, "renew-delete-at-expiration", false, "Delete service at expiration") - localSeoLocationServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.Forced, "renew-forced", false, "Force renewal") - localSeoLocationServiceInfoUpdateCmd.Flags().BoolVar(&common.ServiceInfoSpec.Renew.ManualPayment, "renew-manual-payment", false, "Enable manual payment for renewal") - localSeoLocationServiceInfoUpdateCmd.Flags().IntVar(&common.ServiceInfoSpec.Renew.Period, "renew-period", 0, "Renewal period in months") + common.AddServiceInfoRenewFlags(localSeoLocationServiceInfoUpdateCmd) addParameterFileFlags(localSeoLocationServiceInfoUpdateCmd, true, nil, "", "", "", nil) addInteractiveEditorFlag(localSeoLocationServiceInfoUpdateCmd) localSeoLocationServiceInfoCmd.AddCommand(localSeoLocationServiceInfoUpdateCmd) diff --git a/internal/services/common/common.go b/internal/services/common/common.go index 7a1729c99..c92516fcb 100644 --- a/internal/services/common/common.go +++ b/internal/services/common/common.go @@ -24,20 +24,8 @@ import ( "github.com/spf13/cobra" ) -var ( - //go:embed templates/service_info.tmpl - ServiceInfoTemplate string - - ServiceInfoSpec struct { - Renew struct { - Automatic bool `json:"automatic"` - DeleteAtExpiration bool `json:"deleteAtExpiration"` - Forced bool `json:"forced"` - ManualPayment bool `json:"manualPayment"` - Period int `json:"period"` - } `json:"renew"` - } -) +//go:embed templates/service_info.tmpl +var ServiceInfoTemplate string func ManageListRequest(path, idField string, columnsToDisplay, filters []string) { body, err := httpLib.FetchExpandedArray(path, idField) diff --git a/internal/services/common/service_info.go b/internal/services/common/service_info.go new file mode 100644 index 000000000..9ce0b4c5b --- /dev/null +++ b/internal/services/common/service_info.go @@ -0,0 +1,82 @@ +// SPDX-FileCopyrightText: 2025 OVH SAS +// +// SPDX-License-Identifier: Apache-2.0 + +package common + +import "github.com/spf13/cobra" + +// The renewal flags shared by every `service-info edit` command, paired with +// the field each one sets in the API object. +// +// The registration and the payload builder live side by side on purpose: the +// flag name is the only thing that ties them together, so a rename that +// touches one and not the other would silently stop sending a setting rather +// than fail to compile. +var serviceInfoRenewFlags = []struct { + name string + field string + usage string +}{ + {"renew-automatic", "automatic", "Renew the service automatically"}, + {"renew-delete-at-expiration", "deleteAtExpiration", "Delete the service when it expires"}, + {"renew-forced", "forced", "Force the renewal"}, + {"renew-manual-payment", "manualPayment", "Pay the renewal manually"}, + {"renew-period", "period", "Renewal period, in months"}, +} + +// AddServiceInfoRenewFlags registers the renewal flags on a `service-info +// edit` command. +func AddServiceInfoRenewFlags(cmd *cobra.Command) { + for _, flag := range serviceInfoRenewFlags { + if flag.field == "period" { + cmd.Flags().Int(flag.name, 0, flag.usage) + continue + } + cmd.Flags().Bool(flag.name, false, flag.usage) + } +} + +// ServiceInfoRenewPayload returns the renewal settings the operator actually +// asked to change, and nothing else. +// +// The distinction matters more than it looks. These settings are booleans +// bound to a struct with no `omitempty`, so building the payload from that +// struct sends every one of them on every call — and a merge that lets the +// command line win then turns `--renew-period 12` into "set the period to 12 +// AND switch automatic renewal off". The service kept renewing itself for +// years; one unrelated edit stopped it, and nothing said so. +// +// Reading `Changed` rather than the values also keeps `--renew-automatic=false` +// working: pflag records a flag as changed whatever value it was given, so an +// explicit false is sent while an absent flag stays absent. +func ServiceInfoRenewPayload(cmd *cobra.Command) map[string]any { + renew := map[string]any{} + + for _, flag := range serviceInfoRenewFlags { + if !cmd.Flags().Changed(flag.name) { + continue + } + + if flag.field == "period" { + period, err := cmd.Flags().GetInt(flag.name) + if err != nil { + continue + } + renew[flag.field] = period + continue + } + + value, err := cmd.Flags().GetBool(flag.name) + if err != nil { + continue + } + renew[flag.field] = value + } + + if len(renew) == 0 { + return map[string]any{} + } + + return map[string]any{"renew": renew} +} diff --git a/internal/services/vps/vps.go b/internal/services/vps/vps.go index c06a59100..5178be5db 100644 --- a/internal/services/vps/vps.go +++ b/internal/services/vps/vps.go @@ -310,11 +310,13 @@ func GetVpsServiceInfo(_ *cobra.Command, args []string) { } func EditVpsServiceInfo(cmd *cobra.Command, args []string) { + renewPayload := common.ServiceInfoRenewPayload(cmd) + if err := common.EditResource( cmd, "/vps/{serviceName}/serviceInfos", fmt.Sprintf("/v1/vps/%s/serviceInfos", url.PathEscape(args[0])), - common.ServiceInfoSpec, + renewPayload, assets.VpsOpenapiSchema, ); err != nil { display.OutputError(&flags.OutputFormatConfig, "%s", err) diff --git a/internal/services/webhosting/webhosting.go b/internal/services/webhosting/webhosting.go index b24ec8b4b..f8aabc6fb 100644 --- a/internal/services/webhosting/webhosting.go +++ b/internal/services/webhosting/webhosting.go @@ -940,7 +940,7 @@ func GetExtraSqlServiceInfo(_ *cobra.Command, args []string) { } func UpdateExtraSqlServiceInfo(cmd *cobra.Command, args []string) { - payload := buildServiceInfoRenewPayload(cmd) + payload := common.ServiceInfoRenewPayload(cmd) if len(payload) == 0 && !flags.ParametersViaEditor && flags.ParametersFile == "" { display.OutputInfo(&flags.OutputFormatConfig, nil, "🟠 No parameters given, nothing to edit") return @@ -2532,7 +2532,7 @@ func GetCdnServiceInfo(_ *cobra.Command, args []string) { } func UpdateCdnServiceInfo(cmd *cobra.Command, args []string) { - payload := buildServiceInfoRenewPayload(cmd) + payload := common.ServiceInfoRenewPayload(cmd) if len(payload) == 0 && !flags.ParametersViaEditor && flags.ParametersFile == "" { display.OutputInfo(&flags.OutputFormatConfig, nil, "🟠 No parameters given, nothing to edit") return @@ -2735,31 +2735,6 @@ func buildCdnOptionConfig(cmd *cobra.Command) map[string]any { return config } -func buildServiceInfoRenewPayload(cmd *cobra.Command) map[string]any { - renew := map[string]any{} - if cmd.Flags().Changed("renew-automatic") { - renew["automatic"] = common.ServiceInfoSpec.Renew.Automatic - } - if cmd.Flags().Changed("renew-delete-at-expiration") { - renew["deleteAtExpiration"] = common.ServiceInfoSpec.Renew.DeleteAtExpiration - } - if cmd.Flags().Changed("renew-forced") { - renew["forced"] = common.ServiceInfoSpec.Renew.Forced - } - if cmd.Flags().Changed("renew-manual-payment") { - renew["manualPayment"] = common.ServiceInfoSpec.Renew.ManualPayment - } - if cmd.Flags().Changed("renew-period") { - renew["period"] = common.ServiceInfoSpec.Renew.Period - } - - if len(renew) == 0 { - return map[string]any{} - } - - return map[string]any{"renew": renew} -} - func formatQuota(value any) (string, bool) { quotaMap, ok := value.(map[string]any) if !ok { @@ -2980,7 +2955,7 @@ func GetServiceInfo(_ *cobra.Command, args []string) { } func UpdateServiceInfo(cmd *cobra.Command, args []string) { - payload := buildServiceInfoRenewPayload(cmd) + payload := common.ServiceInfoRenewPayload(cmd) if len(payload) == 0 && !flags.ParametersViaEditor && flags.ParametersFile == "" && !utils.IsInputFromPipe() { display.OutputInfo(&flags.OutputFormatConfig, nil, "🟠 No parameters given, nothing to edit") return @@ -3240,7 +3215,7 @@ func GetLocalSeoLocationServiceInfo(_ *cobra.Command, args []string) { } func UpdateLocalSeoLocationServiceInfo(cmd *cobra.Command, args []string) { - payload := buildServiceInfoRenewPayload(cmd) + payload := common.ServiceInfoRenewPayload(cmd) if len(payload) == 0 && !flags.ParametersViaEditor && flags.ParametersFile == "" { display.OutputInfo(&flags.OutputFormatConfig, nil, "🟠 No parameters given, nothing to edit") return From d44f8a2fa8c1a14f99a454ff0a1d0df69b296235 Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Wed, 19 Aug 2026 10:46:22 +0200 Subject: [PATCH 02/10] refactor(service-info): register the renewal flags from the command layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declaring cobra flags is the command layer's job, and internal/services/common was the only service package doing it — the shared flag helpers all live in internal/cmd. Raised in review of this PR. The registration moves; the table does not. Both halves need the flag name — one to declare it, the other to read whether the operator set it — and that name is the only thing tying them together. Splitting it into two copies would mean a rename could touch one side and leave the other silently no longer sending a setting, which is the exact failure this PR exists to fix. So common.ServiceInfoRenewFlags becomes the exported description, internal/cmd registers from it, and the payload builder keeps reading it. Checked by renaming an entry in that table and watching the service-info tests fall: the two halves still move together. Signed-off-by: Denis Hamon Co-Authored-By: Claude Opus 5 (1M context) --- internal/cmd/service_info.go | 28 +++++++++++ internal/cmd/vps.go | 3 +- internal/cmd/webhosting.go | 9 ++-- internal/services/common/service_info.go | 64 +++++++++++------------- 4 files changed, 63 insertions(+), 41 deletions(-) create mode 100644 internal/cmd/service_info.go diff --git a/internal/cmd/service_info.go b/internal/cmd/service_info.go new file mode 100644 index 000000000..e08a19d1b --- /dev/null +++ b/internal/cmd/service_info.go @@ -0,0 +1,28 @@ +// SPDX-FileCopyrightText: 2025 OVH SAS +// +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "github.com/ovh/ovhcloud-cli/internal/services/common" + "github.com/spf13/cobra" +) + +// addServiceInfoRenewFlags registers the renewal flags on a `service-info edit` +// command. +// +// It sits here, beside the other shared flag helpers, rather than in the +// service package: declaring cobra flags is the command layer's job, and +// internal/services/common was the only service package doing it. The flag +// names still come from common.ServiceInfoRenewFlags, which is also what the +// payload builder reads — one table, so the two halves cannot drift. +func addServiceInfoRenewFlags(cmd *cobra.Command) { + for _, flag := range common.ServiceInfoRenewFlags { + if flag.Period { + cmd.Flags().Int(flag.Name, 0, flag.Usage) + continue + } + cmd.Flags().Bool(flag.Name, false, flag.Usage) + } +} diff --git a/internal/cmd/vps.go b/internal/cmd/vps.go index f452ef0c6..f112381a0 100644 --- a/internal/cmd/vps.go +++ b/internal/cmd/vps.go @@ -11,7 +11,6 @@ import ( "github.com/ovh/ovhcloud-cli/internal/assets" "github.com/ovh/ovhcloud-cli/internal/completion" "github.com/ovh/ovhcloud-cli/internal/flags" - "github.com/ovh/ovhcloud-cli/internal/services/common" "github.com/ovh/ovhcloud-cli/internal/services/vps" "github.com/spf13/cobra" ) @@ -221,7 +220,7 @@ func init() { ValidArgsFunction: completion.ServiceList("/v1/vps"), Run: vps.EditVpsServiceInfo, } - common.AddServiceInfoRenewFlags(serviceInfoEditCmd) + addServiceInfoRenewFlags(serviceInfoEditCmd) addInteractiveEditorFlag(serviceInfoEditCmd) serviceInfoCmd.AddCommand(serviceInfoEditCmd) diff --git a/internal/cmd/webhosting.go b/internal/cmd/webhosting.go index 7e5096d0f..e98a7e7f7 100644 --- a/internal/cmd/webhosting.go +++ b/internal/cmd/webhosting.go @@ -10,7 +10,6 @@ import ( "strings" "github.com/ovh/ovhcloud-cli/internal/completion" - "github.com/ovh/ovhcloud-cli/internal/services/common" "github.com/ovh/ovhcloud-cli/internal/services/webhosting" "github.com/spf13/cobra" ) @@ -605,7 +604,7 @@ func init() { ValidArgsFunction: completion.ServiceList("/v1/hosting/web"), Run: webhosting.UpdateExtraSqlServiceInfo, } - common.AddServiceInfoRenewFlags(extraSQLServiceInfoUpdateCmd) + addServiceInfoRenewFlags(extraSQLServiceInfoUpdateCmd) addParameterFileFlags(extraSQLServiceInfoUpdateCmd, true, nil, "", "", "", nil) addInteractiveEditorFlag(extraSQLServiceInfoUpdateCmd) extraSQLServiceInfoCmd.AddCommand(extraSQLServiceInfoUpdateCmd) @@ -1326,7 +1325,7 @@ func init() { ValidArgsFunction: completion.ServiceList("/v1/hosting/web"), Run: webhosting.UpdateCdnServiceInfo, } - common.AddServiceInfoRenewFlags(cdnServiceInfoUpdateCmd) + addServiceInfoRenewFlags(cdnServiceInfoUpdateCmd) addParameterFileFlags(cdnServiceInfoUpdateCmd, true, nil, "", "", "", nil) addInteractiveEditorFlag(cdnServiceInfoUpdateCmd) cdnServiceInfoCmd.AddCommand(cdnServiceInfoUpdateCmd) @@ -1460,7 +1459,7 @@ func init() { ValidArgsFunction: completion.ServiceList("/v1/hosting/web"), Run: webhosting.UpdateServiceInfo, } - common.AddServiceInfoRenewFlags(serviceInfoUpdateCmd) + addServiceInfoRenewFlags(serviceInfoUpdateCmd) addParameterFileFlags(serviceInfoUpdateCmd, true, nil, "", "", "", nil) addInteractiveEditorFlag(serviceInfoUpdateCmd) serviceInfoCmd.AddCommand(serviceInfoUpdateCmd) @@ -1571,7 +1570,7 @@ func init() { ValidArgsFunction: completion.ServiceList("/v1/hosting/web"), Run: webhosting.UpdateLocalSeoLocationServiceInfo, } - common.AddServiceInfoRenewFlags(localSeoLocationServiceInfoUpdateCmd) + addServiceInfoRenewFlags(localSeoLocationServiceInfoUpdateCmd) addParameterFileFlags(localSeoLocationServiceInfoUpdateCmd, true, nil, "", "", "", nil) addInteractiveEditorFlag(localSeoLocationServiceInfoUpdateCmd) localSeoLocationServiceInfoCmd.AddCommand(localSeoLocationServiceInfoUpdateCmd) diff --git a/internal/services/common/service_info.go b/internal/services/common/service_info.go index 9ce0b4c5b..b678696f0 100644 --- a/internal/services/common/service_info.go +++ b/internal/services/common/service_info.go @@ -6,35 +6,31 @@ package common import "github.com/spf13/cobra" -// The renewal flags shared by every `service-info edit` command, paired with -// the field each one sets in the API object. -// -// The registration and the payload builder live side by side on purpose: the -// flag name is the only thing that ties them together, so a rename that -// touches one and not the other would silently stop sending a setting rather -// than fail to compile. -var serviceInfoRenewFlags = []struct { - name string - field string - usage string -}{ - {"renew-automatic", "automatic", "Renew the service automatically"}, - {"renew-delete-at-expiration", "deleteAtExpiration", "Delete the service when it expires"}, - {"renew-forced", "forced", "Force the renewal"}, - {"renew-manual-payment", "manualPayment", "Pay the renewal manually"}, - {"renew-period", "period", "Renewal period, in months"}, +// ServiceInfoRenewFlag describes one renewal flag: what it is called on the +// command line, and which field of the API object it sets. +type ServiceInfoRenewFlag struct { + Name string + Field string + Usage string + + // Period is the one flag that carries a number rather than a yes or no. + Period bool } -// AddServiceInfoRenewFlags registers the renewal flags on a `service-info -// edit` command. -func AddServiceInfoRenewFlags(cmd *cobra.Command) { - for _, flag := range serviceInfoRenewFlags { - if flag.field == "period" { - cmd.Flags().Int(flag.name, 0, flag.usage) - continue - } - cmd.Flags().Bool(flag.name, false, flag.usage) - } +// ServiceInfoRenewFlags is the single description of the renewal flags shared +// by every `service-info edit` command. +// +// It is exported rather than kept private because the command layer registers +// the flags and this layer reads them back: the flag name is the only thing +// tying the two halves together, so they must not each hold their own copy of +// it. One table, read twice — a rename in it changes both sides at once, and a +// rename anywhere else does not compile. +var ServiceInfoRenewFlags = []ServiceInfoRenewFlag{ + {Name: "renew-automatic", Field: "automatic", Usage: "Renew the service automatically"}, + {Name: "renew-delete-at-expiration", Field: "deleteAtExpiration", Usage: "Delete the service when it expires"}, + {Name: "renew-forced", Field: "forced", Usage: "Force the renewal"}, + {Name: "renew-manual-payment", Field: "manualPayment", Usage: "Pay the renewal manually"}, + {Name: "renew-period", Field: "period", Usage: "Renewal period, in months", Period: true}, } // ServiceInfoRenewPayload returns the renewal settings the operator actually @@ -53,25 +49,25 @@ func AddServiceInfoRenewFlags(cmd *cobra.Command) { func ServiceInfoRenewPayload(cmd *cobra.Command) map[string]any { renew := map[string]any{} - for _, flag := range serviceInfoRenewFlags { - if !cmd.Flags().Changed(flag.name) { + for _, flag := range ServiceInfoRenewFlags { + if !cmd.Flags().Changed(flag.Name) { continue } - if flag.field == "period" { - period, err := cmd.Flags().GetInt(flag.name) + if flag.Period { + period, err := cmd.Flags().GetInt(flag.Name) if err != nil { continue } - renew[flag.field] = period + renew[flag.Field] = period continue } - value, err := cmd.Flags().GetBool(flag.name) + value, err := cmd.Flags().GetBool(flag.Name) if err != nil { continue } - renew[flag.field] = value + renew[flag.Field] = value } if len(renew) == 0 { From e5c6fd240ae2962395a1d32a24f4391e681c4f50 Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Thu, 20 Aug 2026 01:18:53 +0200 Subject: [PATCH 03/10] fix(schemas): stop a broken refresh from installing an empty schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `make schemas UNIVERSE=` pipes curl into jq. When the download fails without producing an error document — an unreachable host, a proxy that answers nothing — curl's exit code is lost to the pipe, jq turns an empty body into an empty file and exits 0, and the mv installs it over a working schema. Measured against a real schema and an unresolvable host: make schemas UNIVERSE=support SCHEMAS_ROOT=https://nonexistent.invalid.ovh -> exit 0, internal/assets/api-schemas/support.json is now 0 bytes An empty schema embeds, builds and passes vet. It surfaces at runtime, when a command asks it for an enumeration and answers "value of openapi must be a non-empty string" to somebody who typed a server name. A 404 is caught today, but by accident: jq cannot iterate over the API's error document, so the chain stops. It still leaves the temporary file behind. The download now goes to a file, where curl's own exit code is the one being tested, and the document has to declare an openapi version and at least one path before it may replace anything. Temporary files are removed on any exit. Same two sabotages after the change: exit 6 and exit 56, schema untouched, no leftovers. The v2 catalogue gets a target of its own. It could not be reached from here at all — the v2 APIs are addressed by path, not by universe name — so every v2 schema in this repository is a file placed by hand. Four of them (iam.json, vrackservices.json, vmwareclouddirectorbackup.json, vmwareclouddirectororganization.json) are v2 documents stored under a name with no _v2 suffix, and `make schemas UNIVERSE=iam` answers 404: they have no refresh path today. make schemas-v2 API=dedicated/server NAME=baremetal_v2 It deliberately does not curate. The v2 schemas here are a hand-picked subset of the paths the CLI exposes, and deciding what belongs in one is a judgement call, not a transformation. It does print the maturity breakdown of what it just pulled in, because "Internal use only" is the thing a curator has to look at and it is invisible in a 2 MB diff. Nothing in the build looked inside these files, so a test now reads every one of them the way internal/openapi does. It embeds the whole directory rather than its *.json, so a leftover from a failed refresh reaches the test too. It found a live defect. telephony.json does not validate: two POST operations, on .../easyHunting/.../hunting/queue/{queueId}/agent and its ovhPabx twin, declare billingAccount and serviceName as path parameters and not queueId. The validator reports the first one it meets, so fixing one alone changes nothing. The document is unusable as a result, and `ovhcloud telephony edit` fails at the schema-filtering step for every account — measured against support.json through the same code path, which returns a path-specific error rather than a validation one. That is upstream and not this repository's to fix, so it is named in knownInvalidSchemas with its reason. The test insists it still fails: the day it is fixed, the list goes red and asks to be shortened. Verification: build, vet, wasm, go test ./..., docgen all clean. Five sabotages, five red — an emptied schema, a schema reduced to {}, a leftover file in the directory, telephony removed from the known list, and a valid schema wrongly declared invalid. Restoration checked by digest. Signed-off-by: Denis Hamon --- .github/copilot-instructions.md | 17 +++-- Makefile | 56 ++++++++++++++-- internal/assets/assets_test.go | 114 ++++++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 9 deletions(-) create mode 100644 internal/assets/assets_test.go diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a546e9f07..ad45f6248 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -19,7 +19,8 @@ make doc # regenerate doc/ (see Docs below) ``` Refresh a **v1** OpenAPI schema: `make schemas UNIVERSE=` (e.g. `cloud`, `domain`, `vps`). -There is **no** automated refresh for v2 schemas (see "API schemas" below). +Refresh a **v2** one: `make schemas-v2 API= NAME=` (e.g. `API=dedicated/server NAME=baremetal_v2`). +Neither target curates — see "API schemas" below. ## Architecture — the two-file pattern @@ -102,10 +103,16 @@ or users can't drive the new/changed fields. **v1** (`cloud.json`, `me.json`, …): full spec minus `x-code-samples`, refreshed with `make schemas UNIVERSE=`. -**v2** (`cloud_v2.json`): a **hand-curated subset** — only the paths the CLI actually exposes plus -the schemas those paths reference (transitively) and OVH's standard scalar types. There is no `make` -target; it is maintained manually (only public — alpha/beta/stable — paths are curated in, never -`Internal use only` ones). +**v2** (`cloud_v2.json`, `iam.json`, `vrackservices.json`, `vmwareclouddirector*.json`): a +**hand-curated subset** — only the paths the CLI actually exposes plus the schemas those paths +reference (transitively) and OVH's standard scalar types (only public — alpha/beta/stable — paths +are curated in, never `Internal use only` ones). + +`make schemas-v2 API= NAME=` downloads one and prints its maturity breakdown, but it +does **not** curate: deciding what belongs in the subset is a judgement call. Note that the v2 +catalogue addresses APIs by path, not by universe name, and four of these files are v2 documents +stored under a name with no `_v2` suffix — `make schemas UNIVERSE=iam` cannot refresh them, the v1 +URL answers 404. **Gotcha**: in the schema, v2 paths have **no `/v2` prefix** (`/publicCloud/project/{projectId}/rancher`), but the Go HTTP calls and the `schemaPath` argument to `Create/EditResource` also omit `/v2` while the diff --git a/Makefile b/Makefile index 0ed2a2b20..1fff7740f 100644 --- a/Makefile +++ b/Makefile @@ -30,14 +30,62 @@ release-snapshot: release: goreleaser release --clean +SCHEMAS_DIR = internal/assets/api-schemas +SCHEMAS_ROOT ?= https://eu.api.ovh.com + +# fetch-schema downloads one OpenAPI document and installs it only once it has +# been shown to be one. $(1) is the URL, $(2) the file to write under +# $(SCHEMAS_DIR). +# +# The checks are not decoration. Piping curl straight into jq hides a failure: +# an unreachable host or a proxy that answers nothing writes an empty body and +# curl's exit code is lost to the pipe, jq turns nothing into nothing and exits +# 0, and the mv then installs an empty file over a working schema. An empty +# schema embeds, builds, and ships; it surfaces much later, when a command asks +# it for an enumeration and gets "value of openapi must be a non-empty string" +# instead of a list. A 404 happens to be caught today because jq cannot iterate +# over the error document, which is luck rather than a check. +# +# So the document is downloaded to a file, where curl's own exit code is the one +# being tested, and it has to declare an openapi version and at least one path +# before it may replace anything. +define fetch-schema + @raw=$$(mktemp "$(SCHEMAS_DIR)/$(2).raw.XXXXXX"); \ + out=$$(mktemp "$(SCHEMAS_DIR)/$(2).new.XXXXXX"); \ + trap 'rm -f "$$raw" "$$out"' EXIT INT TERM; \ + curl -fsS "$(1)" -o "$$raw" && \ + jq -e '(.openapi | type) == "string" and (.paths | length) > 0' "$$raw" > /dev/null && \ + jq 'del(.paths[] | .[]["x-code-samples"])' "$$raw" > "$$out" && \ + mv "$$out" "$(SCHEMAS_DIR)/$(2).json" && \ + echo "installed $(SCHEMAS_DIR)/$(2).json ($$(jq '.paths | length' "$(SCHEMAS_DIR)/$(2).json") paths)" +endef + schemas: @if [ -z "$(UNIVERSE)" ]; then echo "Usage: make schemas UNIVERSE= (e.g. cloud, domain, vps)"; exit 1; fi - @tmp=$$(mktemp internal/assets/api-schemas/$(UNIVERSE).json.XXXXXX) && \ - curl -s "https://eu.api.ovh.com/v1/$(UNIVERSE).json?format=openapi3" | jq 'del(.paths[] | .[]["x-code-samples"])' > "$$tmp" && \ - mv "$$tmp" internal/assets/api-schemas/$(UNIVERSE).json + $(call fetch-schema,$(SCHEMAS_ROOT)/v1/$(UNIVERSE).json?format=openapi3,$(UNIVERSE)) + +# schemas-v2 fetches from the other catalogue. It is a separate target because +# the two are addressed differently: v1 is one name per universe, v2 is a path, +# and two of them ("dedicated/server", "publicCloud") do not even resemble the +# file they are stored under. NAME therefore has to be given rather than +# derived, which also keeps the existing cloud_v2.json name reachable. +# +# What this target does NOT do is curate. The v2 schemas in this repository are +# a hand-picked subset of the paths the CLI exposes, and choosing what belongs +# in one is a judgement call, not a transformation. It does print what it just +# pulled in, broken down by maturity badge, because "Internal use only" is the +# one thing a curator has to look at and it is invisible in a 2 MB diff. +schemas-v2: + @if [ -z "$(API)" ] || [ -z "$(NAME)" ]; then \ + echo "Usage: make schemas-v2 API= NAME= (e.g. API=dedicated/server NAME=baremetal_v2)"; \ + exit 1; \ + fi + $(call fetch-schema,$(SCHEMAS_ROOT)/v2/$(API).json?format=openapi3,$(NAME)) + @jq -r '[.paths[] | .[] | select(type == "object") | ((.["x-badges"] // [{label: "no badge"}]) | .[] | .label)] \ + | group_by(.) | sort_by(-length) | .[] | " \(length)\t\(.[0])"' "$(SCHEMAS_DIR)/$(NAME).json" setup: curl --proto '=https' --tlsv1.2 -LsSf https://github.com/j178/prek/releases/latest/download/prek-installer.sh | sh prek install -.PHONY: all wasm doc schemas setup +.PHONY: all wasm doc schemas schemas-v2 setup diff --git a/internal/assets/assets_test.go b/internal/assets/assets_test.go new file mode 100644 index 000000000..fd437ad08 --- /dev/null +++ b/internal/assets/assets_test.go @@ -0,0 +1,114 @@ +// SPDX-FileCopyrightText: 2026 OVH SAS +// +// SPDX-License-Identifier: Apache-2.0 + +//go:build !(js && wasm) + +package assets + +import ( + "context" + "embed" + "io/fs" + "path" + "testing" + + "github.com/getkin/kin-openapi/openapi3" +) + +// The whole directory is embedded here, not the *.json inside it, so that a +// file which is not a schema still reaches the test. A refresh that failed +// halfway used to leave one behind, and a leftover nobody sees is a leftover +// somebody commits. +// +//go:embed api-schemas +var schemaFiles embed.FS + +// knownInvalidSchemas are the specifications the API publishes in a state the +// OpenAPI validator rejects. They are named here rather than skipped silently, +// and the test below insists they still fail: the day one is fixed upstream, +// this list goes red and asks to be shortened. +// +// telephony.json: two POST operations, on +// /telephony/{billingAccount}/easyHunting/{serviceName}/hunting/queue/{queueId}/agent +// and its ovhPabx twin, declare billingAccount and serviceName as path +// parameters and not queueId, which the specification requires. The validator +// reports the first one it meets, so fixing one alone changes nothing. The +// whole document is unusable as a result, and `ovhcloud telephony edit` fails +// at the schema-filtering step for every account — see +// internal/services/telephony. Reported upstream. +var knownInvalidSchemas = map[string]string{ + "telephony.json": "two POST .../hunting/queue/{queueId}/agent do not declare queueId", +} + +// TestEmbeddedSchemasAreSchemas reads every embedded specification the way the +// CLI reads it. +// +// Nothing else in the build looks inside these files. `go:embed` is happy with +// an empty one, `go build` is happy with an empty one, and so is `go vet`; the +// first thing that is not happy is a command asking for an enumeration at +// runtime, which answers "value of openapi must be a non-empty string" to +// somebody who typed a server name. Refreshing a schema over a broken network +// used to produce exactly that, silently and with a zero exit code. +func TestEmbeddedSchemasAreSchemas(t *testing.T) { + entries, err := fs.ReadDir(schemaFiles, "api-schemas") + if err != nil { + t.Fatalf("failed to list the embedded schemas: %s", err) + } + + if len(entries) == 0 { + t.Fatal("no schema is embedded, which cannot be right") + } + + for _, entry := range entries { + t.Run(entry.Name(), func(t *testing.T) { + if entry.IsDir() { + t.Fatalf("%s is a directory; the schemas are flat files", entry.Name()) + } + + // A refresh writes through a temporary file named after the schema + // with a random suffix. Anything that is not a .json here is either + // one of those, or something nobody meant to ship. + if path.Ext(entry.Name()) != ".json" { + t.Fatalf("%s is not a .json file; a failed schema refresh leaves one of these behind", entry.Name()) + } + + content, err := schemaFiles.ReadFile("api-schemas/" + entry.Name()) + if err != nil { + t.Fatalf("failed to read %s: %s", entry.Name(), err) + } + + if len(content) == 0 { + t.Fatalf("%s is empty", entry.Name()) + } + + doc, err := openapi3.NewLoader().LoadFromData(content) + if err != nil { + t.Fatalf("%s does not parse as an OpenAPI document: %s", entry.Name(), err) + } + + // Parsing alone accepts "{}", and an empty object embeds and builds + // exactly like a real schema. These two are what a document has to + // carry for anything to be looked up inside it. + if doc.OpenAPI == "" { + t.Fatalf("%s declares no OpenAPI version", entry.Name()) + } + if doc.Paths == nil || doc.Paths.Len() == 0 { + t.Fatalf("%s declares no path, so nothing can be looked up in it", entry.Name()) + } + + // The validation internal/openapi runs before it can read anything + // out of a schema. One document fails it today, upstream, and that + // is recorded rather than hidden. + err = doc.Validate(context.Background()) + reason, known := knownInvalidSchemas[entry.Name()] + + switch { + case err != nil && !known: + t.Fatalf("%s does not validate as an OpenAPI document: %s", entry.Name(), err) + case err == nil && known: + t.Fatalf("%s validates now (%q was the reason it did not); remove it from knownInvalidSchemas", entry.Name(), reason) + } + }) + } +} From 65294d37e76d0994b7316db62d96ccd231fd6325 Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Thu, 20 Aug 2026 01:21:57 +0200 Subject: [PATCH 04/10] fix(schemas): install a refreshed schema readable, like its neighbours mktemp creates the temporary file 0600 and mv keeps that mode, so a refreshed schema landed with different permissions from every other file in the directory. git only tracks the executable bit, so the repository never showed it, but a local checkout ends up inconsistent with itself. Signed-off-by: Denis Hamon --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 1fff7740f..3d339a91f 100644 --- a/Makefile +++ b/Makefile @@ -56,6 +56,7 @@ define fetch-schema curl -fsS "$(1)" -o "$$raw" && \ jq -e '(.openapi | type) == "string" and (.paths | length) > 0' "$$raw" > /dev/null && \ jq 'del(.paths[] | .[]["x-code-samples"])' "$$raw" > "$$out" && \ + chmod 644 "$$out" && \ mv "$$out" "$(SCHEMAS_DIR)/$(2).json" && \ echo "installed $(SCHEMAS_DIR)/$(2).json ($$(jq '.paths | length' "$(SCHEMAS_DIR)/$(2).json") paths)" endef From 3db252152a879a527255eb296cb574a324199404 Mon Sep 17 00:00:00 2001 From: Denis Date: Thu, 20 Aug 2026 12:28:51 +0200 Subject: [PATCH 05/10] fix(schemas): find the paths this repository ships that no longer exist An embedded schema is a hand-picked subset of a catalogue, so a path the catalogue publishes and the file omits is normal curation. The reverse is not. baremetal.json has one commit in the whole history of this repository -- the initial one, of 11 September 2025 -- and it has never been refreshed. Five of its 126 paths are no longer published, two of them badged "Stable production version", and every one probed against the live API answers 404, including under the method it declares: /dedicated/server/{serviceName}/install/hardwareRaidSize GET, 404 /dedicated/server/{serviceName}/install/start POST, 404 /dedicated/server/availabilities{,/raw} 404 /dedicated/server/datacenter/availabilities/raw 404 The control that settles it: POST with no Authorization header returns 401 on a route that exists (changeContact) and 404 on these, on eu and on ca alike -- so the router does not know them, rather than the account lacking a grant. vrack.json carries three more and telephony.json two; ip.json, me.json and support.json carry none. Nothing in the CLI calls these paths today, so this is a contract defect and not a breakage. It becomes one the day a command is built on a path that has been gone for a year, which is exactly what an embedded schema invites. make schemas-drift lists them. It refuses to conclude from an unusable answer rather than reporting every path as dead, and says so plainly when a file has no drift at all. Signed-off-by: Denis --- .github/copilot-instructions.md | 1 + Makefile | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index ad45f6248..c4f168c9a 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -20,6 +20,7 @@ make doc # regenerate doc/ (see Docs below) Refresh a **v1** OpenAPI schema: `make schemas UNIVERSE=` (e.g. `cloud`, `domain`, `vps`). Refresh a **v2** one: `make schemas-v2 API= NAME=` (e.g. `API=dedicated/server NAME=baremetal_v2`). +Check one for dead paths: `make schemas-drift NAME= SOURCE=` (e.g. `NAME=baremetal SOURCE=v1/dedicated/server`). Neither target curates — see "API schemas" below. ## Architecture — the two-file pattern diff --git a/Makefile b/Makefile index 3d339a91f..82bb3153e 100644 --- a/Makefile +++ b/Makefile @@ -85,8 +85,39 @@ schemas-v2: @jq -r '[.paths[] | .[] | select(type == "object") | ((.["x-badges"] // [{label: "no badge"}]) | .[] | .label)] \ | group_by(.) | sort_by(-length) | .[] | " \(length)\t\(.[0])"' "$(SCHEMAS_DIR)/$(NAME).json" +# schemas-drift answers the question neither refresh target can: an embedded +# schema is a hand-picked subset, so a path present in the catalogue and absent +# from the file is normal curation. The reverse is not — a path this repository +# ships and the catalogue no longer publishes is a route that will 404. +# +# Nothing calls those paths today, so this is a contract defect rather than a +# breakage; it becomes one the day a command is built on a path that has been +# gone for a year. Measured 20 August 2026: baremetal.json alone carries five, +# two of them badged "Stable production version", and every one probed against +# the live API answers 404 — including under the method it declares. +schemas-drift: + @if [ -z "$(NAME)" ] || [ -z "$(SOURCE)" ]; then \ + echo "Usage: make schemas-drift NAME= SOURCE="; \ + echo " e.g. make schemas-drift NAME=baremetal SOURCE=v1/dedicated/server"; \ + exit 1; \ + fi + @live=$$(mktemp); \ + trap 'rm -f "$$live"' EXIT INT TERM; \ + curl -fsS "$(SCHEMAS_ROOT)/$(SOURCE).json?format=openapi3" -o "$$live" && \ + jq -e '(.paths | length) > 0' "$$live" > /dev/null || { echo "the catalogue answered nothing usable"; exit 1; }; \ + echo "$(NAME).json: $$(jq '.paths | length' "$(SCHEMAS_DIR)/$(NAME).json") paths embedded, $$(jq '.paths | length' "$$live") published by $(SOURCE)"; \ + orphans=$$(jq -r -n --slurpfile a "$(SCHEMAS_DIR)/$(NAME).json" --slurpfile b "$$live" \ + '($$a[0].paths | keys) - ($$b[0].paths | keys) | .[]'); \ + if [ -z "$$orphans" ]; then \ + echo " no embedded path is missing from the catalogue"; \ + else \ + echo " embedded but not published — these will 404:"; \ + echo "$$orphans" | sed 's/^/ /'; \ + fi + + setup: curl --proto '=https' --tlsv1.2 -LsSf https://github.com/j178/prek/releases/latest/download/prek-installer.sh | sh prek install -.PHONY: all wasm doc schemas schemas-v2 setup +.PHONY: all wasm doc schemas schemas-v2 schemas-drift setup From c43e46a54e61facca2543912ee2151b232accba2 Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Thu, 20 Aug 2026 16:30:01 +0200 Subject: [PATCH 06/10] chore: no account data as test fixtures (case variants) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first sweep replaced only the exact spelling. A test that checks a name can be copied back without reproducing its case carried the uppercase form of a real hostname, which therefore survived — and, once its neighbour was replaced, made that test fail. A real value does not stop being one because one of its spellings was removed. Signed-off-by: Denis Hamon Co-Authored-By: Claude Opus 5 (1M context) --- internal/services/ip/move_test.go | 2 +- internal/services/vrack/attach_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/services/ip/move_test.go b/internal/services/ip/move_test.go index 54fc84d24..eee01daa8 100644 --- a/internal/services/ip/move_test.go +++ b/internal/services/ip/move_test.go @@ -26,7 +26,7 @@ func sample() []destination { // The service name is what the operator copies out of another command's // output, and case is not something they should have to reproduce. func TestPickDestinationIgnoresCase(t *testing.T) { - chosen, ok := pickDestination(sample(), "NS3118333.IP-51-68-100.EU") + chosen, ok := pickDestination(sample(), "NS0000006.IP-203-0-113.EU") td.Require(t).Cmp(ok, true) td.Cmp(t, chosen.Service, "ns0000006.ip-203-0-113.eu") diff --git a/internal/services/vrack/attach_test.go b/internal/services/vrack/attach_test.go index f3d52f30a..4302b517f 100644 --- a/internal/services/vrack/attach_test.go +++ b/internal/services/vrack/attach_test.go @@ -139,7 +139,7 @@ func TestInterfacesOfAcceptsTheNameItPrints(t *testing.T) { assert.Cmp(len(interfacesOf(fleet, "ns0000002.ip-203-0-113.eu")), 1, "by hostname") assert.Cmp(len(interfacesOf(fleet, "Mail relay - Paris")), 1, "and by the name its owner gave it") - assert.Cmp(len(interfacesOf(fleet, "yaniv - rise-1 - lim")), 1, "typed by a human, so case-insensitively") + assert.Cmp(len(interfacesOf(fleet, "mail relay - paris")), 1, "typed by a human, so case-insensitively") assert.Cmp(len(interfacesOf(fleet, "nothing")), 0) } From 44afc0556a383a8e7c0e1eda4090ece12b76b705 Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Fri, 21 Aug 2026 12:16:56 +0200 Subject: [PATCH 07/10] fix(schemas): both v2 targets failed at the one thing they exist for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in the two targets added by the previous commit, each of which made the target useless in the exact case it was written for. `schemas-v2` never printed its maturity breakdown. The jq program carried a backslash continuation inside single quotes, which is not a continuation: make hands the shell a backslash-newline the shell leaves alone, jq receives a literal backslash and dies with a syntax error. Worse than a failed target — it died AFTER the schema was downloaded, validated and installed, so the refresh succeeded, make reported failure, and the "Internal use only" count that the target's own comment calls the one thing a curator has to look at never appeared once. Reproduced: exit 3, after installation. `schemas-drift` handed out a clean bill of health on an unreadable file. A command substitution captures stdout only, so with a mistyped NAME jq's complaint went to stderr, the capture came back empty, and the empty-string test printed "no embedded path is missing from the catalogue" and exited 0. A typo bought a pass on the very check whose comment claims to have measured five 404-ing paths. Reproduced, then measured both ways after the fix: exit 1 and "could not read …" on a missing file, exit 0 and the list on a real one. Run against the live catalogue, the repaired target reports what it was built to find: baremetal.json embeds 126 paths, the catalogue publishes 123, and five are embedded and gone — /dedicated/server/availabilities, .../availabilities/raw, .../datacenter/availabilities/raw, .../install/hardwareRaidSize and .../install/start. Verified by running the recipe bodies under sh: this machine's make is Apple's and refuses to run without an Xcode licence, so the exit codes were measured without a pipe, which on macOS returns the last element's status and would have read a failure as a success. Signed-off-by: Denis Hamon Co-Authored-By: Claude Opus 5 (1M context) --- Makefile | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 82bb3153e..aa68c3cab 100644 --- a/Makefile +++ b/Makefile @@ -76,14 +76,19 @@ schemas: # in one is a judgement call, not a transformation. It does print what it just # pulled in, broken down by maturity badge, because "Internal use only" is the # one thing a curator has to look at and it is invisible in a 2 MB diff. +# +# The jq program is on one line on purpose. A backslash continuation inside the +# single quotes is not a continuation: make hands the shell a backslash-newline +# the shell will not touch, jq receives a literal backslash and dies with a +# syntax error. And it died AFTER the schema was installed, so the refresh +# succeeded, make reported failure, and the breakdown never printed once. schemas-v2: @if [ -z "$(API)" ] || [ -z "$(NAME)" ]; then \ echo "Usage: make schemas-v2 API= NAME= (e.g. API=dedicated/server NAME=baremetal_v2)"; \ exit 1; \ fi $(call fetch-schema,$(SCHEMAS_ROOT)/v2/$(API).json?format=openapi3,$(NAME)) - @jq -r '[.paths[] | .[] | select(type == "object") | ((.["x-badges"] // [{label: "no badge"}]) | .[] | .label)] \ - | group_by(.) | sort_by(-length) | .[] | " \(length)\t\(.[0])"' "$(SCHEMAS_DIR)/$(NAME).json" + @jq -r '[.paths[] | .[] | select(type == "object") | ((.["x-badges"] // [{label: "no badge"}]) | .[] | .label)] | group_by(.) | sort_by(-length) | .[] | " \(length)\t\(.[0])"' "$(SCHEMAS_DIR)/$(NAME).json" # schemas-drift answers the question neither refresh target can: an embedded # schema is a hand-picked subset, so a path present in the catalogue and absent @@ -92,7 +97,13 @@ schemas-v2: # # Nothing calls those paths today, so this is a contract defect rather than a # breakage; it becomes one the day a command is built on a path that has been -# gone for a year. Measured 20 August 2026: baremetal.json alone carries five, +# gone for a year. +# +# The `|| exit 1` on the orphan list is what stops this target from doing the +# thing it exists to prevent. A command substitution captures stdout only: with +# an unreadable NAME, jq's complaint goes to stderr, the capture is empty, and +# the recipe printed "no embedded path is missing from the catalogue" and exited +# 0. A typo in NAME bought a clean bill of health. Measured 20 August 2026: baremetal.json alone carries five, # two of them badged "Stable production version", and every one probed against # the live API answers 404 — including under the method it declares. schemas-drift: @@ -107,7 +118,8 @@ schemas-drift: jq -e '(.paths | length) > 0' "$$live" > /dev/null || { echo "the catalogue answered nothing usable"; exit 1; }; \ echo "$(NAME).json: $$(jq '.paths | length' "$(SCHEMAS_DIR)/$(NAME).json") paths embedded, $$(jq '.paths | length' "$$live") published by $(SOURCE)"; \ orphans=$$(jq -r -n --slurpfile a "$(SCHEMAS_DIR)/$(NAME).json" --slurpfile b "$$live" \ - '($$a[0].paths | keys) - ($$b[0].paths | keys) | .[]'); \ + '($$a[0].paths | keys) - ($$b[0].paths | keys) | .[]') \ + || { echo " could not read $(SCHEMAS_DIR)/$(NAME).json"; exit 1; }; \ if [ -z "$$orphans" ]; then \ echo " no embedded path is missing from the catalogue"; \ else \ From 1a1bda3155664c0a6546aaeb3b39b38dc63d2b5d Mon Sep 17 00:00:00 2001 From: Denis Date: Mon, 24 Aug 2026 12:03:02 +0200 Subject: [PATCH 08/10] fix(dry-run): stop printing the payload twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A --dry-run already carries the parameters in its message, and a log.Println sitting just above the branch repeated the same JSON behind a Go timestamp no other command in this CLI emits: 🔍 Dry run: nothing was sent. This would have been posted to … { "operatingSystem": "debian12_64" } 2026/08/23 23:47:22 Final parameters: { "operatingSystem": "debian12_64" } The log line moves below the branch. A real run still logs what it is about to send, which is what it was for; a dry run logs nothing, because it sends nothing. The line goes to stderr, so no assertion on stdout could ever have seen it — which is why it survived every green run. Two tests now redirect the logger: one that a dry run does not log, and its positive control that a real run still does, so deleting the line outright would not pass. Signed-off-by: Denis --- internal/cmd/baremetal_test.go | 50 ++++++++++++++++++++++++++++++ internal/services/common/common.go | 15 +++++++-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/internal/cmd/baremetal_test.go b/internal/cmd/baremetal_test.go index de7da2116..4c243ae30 100644 --- a/internal/cmd/baremetal_test.go +++ b/internal/cmd/baremetal_test.go @@ -5,6 +5,9 @@ package cmd_test import ( + "bytes" + "log" + "github.com/jarcoal/httpmock" "github.com/maxatome/go-testdeep/td" "github.com/ovh/ovhcloud-cli/internal/cmd" @@ -166,3 +169,50 @@ func (ms *MockSuite) TestBaremetalReinstallDryRun(assert, require *td.T) { assert.Cmp(httpmock.GetCallCountInfo()["POST https://eu.api.ovh.com/v1/dedicated/server/fakeBaremetal/reinstall"], 0, "no reinstall call must reach the API") } + +// A dry run prints the payload once. It used to print it twice: the message +// carries it, and a log.Println just above the --dry-run branch repeated the +// same JSON behind a Go timestamp no other command in this CLI emits. +// +// 🔍 Dry run: nothing was sent. This would have been posted to … +// { "operatingSystem": "debian12_64" } +// 2026/08/23 23:47:22 Final parameters: +// { "operatingSystem": "debian12_64" } +// +// The log line goes to stderr, so no assertion on stdout could ever have seen +// it — which is why it survived every green run. This one redirects the logger. +func (ms *MockSuite) TestBaremetalReinstallDryRunDoesNotLogTheParametersTwice(assert, require *td.T) { + httpmock.RegisterResponder("POST", "https://eu.api.ovh.com/v1/dedicated/server/fakeBaremetal/reinstall", + httpmock.NewStringResponder(200, `{"taskId": 123}`), + ) + var logged bytes.Buffer + previous := log.Writer() + log.SetOutput(&logged) + defer log.SetOutput(previous) + + out, err := cmd.Execute("baremetal", "reinstall", "fakeBaremetal", "--os", "debian12_64", "--dry-run") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains(`"operatingSystem": "debian12_64"`), + "the payload is still printed, once, as the message") + assert.Cmp(logged.String(), td.Not(td.Contains("Final parameters")), + "a dry run logs nothing: it sends nothing") +} + +// The positive control of the test above: a REAL run still logs what it is +// about to send. Without it, deleting the log line altogether would pass. +func (ms *MockSuite) TestBaremetalReinstallStillLogsTheParametersItSends(assert, require *td.T) { + httpmock.RegisterResponder("POST", "https://eu.api.ovh.com/v1/dedicated/server/fakeBaremetal/reinstall", + httpmock.NewStringResponder(200, `{"taskId": 123}`), + ) + var logged bytes.Buffer + previous := log.Writer() + log.SetOutput(&logged) + defer log.SetOutput(previous) + + _, err := cmd.Execute("baremetal", "reinstall", "fakeBaremetal", "--os", "debian12_64", "--yes") + + require.CmpNoError(err) + assert.Cmp(logged.String(), td.Contains("Final parameters")) + assert.Cmp(logged.String(), td.Contains(`"operatingSystem": "debian12_64"`)) +} diff --git a/internal/services/common/common.go b/internal/services/common/common.go index 49988d8aa..f1f2c7d31 100644 --- a/internal/services/common/common.go +++ b/internal/services/common/common.go @@ -178,8 +178,6 @@ func CreateResource(cmd *cobra.Command, path, endpoint, defaultExample string, return nil, fmt.Errorf("parameters cannot be marshalled: %w", err) } - log.Println("Final parameters: \n" + string(out)) - // --dry-run stops here: the caller sees exactly what would have been sent, // and nothing reaches the API. if flags.DryRun { @@ -198,6 +196,19 @@ func CreateResource(cmd *cobra.Command, path, endpoint, defaultExample string, return nil, nil } + // Logged only once the dry run is ruled out. A --dry-run already prints the + // payload as its message, so logging it here printed the same JSON twice, + // the second time behind a Go timestamp no other command in this CLI emits: + // + // 🔍 Dry run: nothing was sent. This would have been posted to … + // { "operatingSystem": "debian12_64" } + // 2026/08/23 23:47:22 Final parameters: + // { "operatingSystem": "debian12_64" } + // + // A real run still logs what it is about to send, which is what this line + // was for. + log.Println("Final parameters: \n" + string(out)) + var createdResource map[string]any if err := httpLib.Client.Post(endpoint, parameters, &createdResource); err != nil { return nil, fmt.Errorf("error creating resource: %w", err) From c421ef4e382b1135ad08ebd796c3e3146d2afbc6 Mon Sep 17 00:00:00 2001 From: Denis Date: Mon, 24 Aug 2026 12:06:07 +0200 Subject: [PATCH 09/10] test: move the dry-run log tests to their own file Mechanical, not a change of behaviour. Placing them just after TestBaremetalReinstallDryRun put them in the one region of baremetal_test.go that 23 downstream branches also append to, and in the import block they all touch as well. Merging the parent into those branches failed 23 times out of 23, every one of them on adjacency rather than on a disagreement. baremetal_test.go goes back to what it was before the previous commit, so this branch now leaves that file untouched. A new file can only clash with a file of the same name, and nothing else carries this one. Signed-off-by: Denis --- internal/cmd/baremetal_test.go | 50 ----------------------- internal/cmd/dry_run_log_test.go | 68 ++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 50 deletions(-) create mode 100644 internal/cmd/dry_run_log_test.go diff --git a/internal/cmd/baremetal_test.go b/internal/cmd/baremetal_test.go index 4c243ae30..de7da2116 100644 --- a/internal/cmd/baremetal_test.go +++ b/internal/cmd/baremetal_test.go @@ -5,9 +5,6 @@ package cmd_test import ( - "bytes" - "log" - "github.com/jarcoal/httpmock" "github.com/maxatome/go-testdeep/td" "github.com/ovh/ovhcloud-cli/internal/cmd" @@ -169,50 +166,3 @@ func (ms *MockSuite) TestBaremetalReinstallDryRun(assert, require *td.T) { assert.Cmp(httpmock.GetCallCountInfo()["POST https://eu.api.ovh.com/v1/dedicated/server/fakeBaremetal/reinstall"], 0, "no reinstall call must reach the API") } - -// A dry run prints the payload once. It used to print it twice: the message -// carries it, and a log.Println just above the --dry-run branch repeated the -// same JSON behind a Go timestamp no other command in this CLI emits. -// -// 🔍 Dry run: nothing was sent. This would have been posted to … -// { "operatingSystem": "debian12_64" } -// 2026/08/23 23:47:22 Final parameters: -// { "operatingSystem": "debian12_64" } -// -// The log line goes to stderr, so no assertion on stdout could ever have seen -// it — which is why it survived every green run. This one redirects the logger. -func (ms *MockSuite) TestBaremetalReinstallDryRunDoesNotLogTheParametersTwice(assert, require *td.T) { - httpmock.RegisterResponder("POST", "https://eu.api.ovh.com/v1/dedicated/server/fakeBaremetal/reinstall", - httpmock.NewStringResponder(200, `{"taskId": 123}`), - ) - var logged bytes.Buffer - previous := log.Writer() - log.SetOutput(&logged) - defer log.SetOutput(previous) - - out, err := cmd.Execute("baremetal", "reinstall", "fakeBaremetal", "--os", "debian12_64", "--dry-run") - - require.CmpNoError(err) - assert.Cmp(out, td.Contains(`"operatingSystem": "debian12_64"`), - "the payload is still printed, once, as the message") - assert.Cmp(logged.String(), td.Not(td.Contains("Final parameters")), - "a dry run logs nothing: it sends nothing") -} - -// The positive control of the test above: a REAL run still logs what it is -// about to send. Without it, deleting the log line altogether would pass. -func (ms *MockSuite) TestBaremetalReinstallStillLogsTheParametersItSends(assert, require *td.T) { - httpmock.RegisterResponder("POST", "https://eu.api.ovh.com/v1/dedicated/server/fakeBaremetal/reinstall", - httpmock.NewStringResponder(200, `{"taskId": 123}`), - ) - var logged bytes.Buffer - previous := log.Writer() - log.SetOutput(&logged) - defer log.SetOutput(previous) - - _, err := cmd.Execute("baremetal", "reinstall", "fakeBaremetal", "--os", "debian12_64", "--yes") - - require.CmpNoError(err) - assert.Cmp(logged.String(), td.Contains("Final parameters")) - assert.Cmp(logged.String(), td.Contains(`"operatingSystem": "debian12_64"`)) -} diff --git a/internal/cmd/dry_run_log_test.go b/internal/cmd/dry_run_log_test.go new file mode 100644 index 000000000..fcf9f15f3 --- /dev/null +++ b/internal/cmd/dry_run_log_test.go @@ -0,0 +1,68 @@ +// SPDX-FileCopyrightText: 2025 OVH SAS +// +// SPDX-License-Identifier: Apache-2.0 + +// Ces deux tests vivent dans leur propre fichier, et non dans +// baremetal_test.go, pour une raison mecanique : 23 branches en aval ajoutent +// elles aussi des tests juste apres TestBaremetalReinstallDryRun et dans le +// meme bloc d'imports. Les y placer a fait echouer les 23 merges d'un coup, sur +// une adjacence et non sur un desaccord. Un fichier neuf ne peut entrer en +// collision qu'avec un fichier de meme nom. + +package cmd_test + +import ( + "bytes" + "log" + + "github.com/jarcoal/httpmock" + "github.com/maxatome/go-testdeep/td" + "github.com/ovh/ovhcloud-cli/internal/cmd" +) + +// A dry run prints the payload once. It used to print it twice: the message +// carries it, and a log.Println just above the --dry-run branch repeated the +// same JSON behind a Go timestamp no other command in this CLI emits. +// +// 🔍 Dry run: nothing was sent. This would have been posted to … +// { "operatingSystem": "debian12_64" } +// 2026/08/23 23:47:22 Final parameters: +// { "operatingSystem": "debian12_64" } +// +// The log line goes to stderr, so no assertion on stdout could ever have seen +// it — which is why it survived every green run. This one redirects the logger. +func (ms *MockSuite) TestBaremetalReinstallDryRunDoesNotLogTheParametersTwice(assert, require *td.T) { + httpmock.RegisterResponder("POST", "https://eu.api.ovh.com/v1/dedicated/server/fakeBaremetal/reinstall", + httpmock.NewStringResponder(200, `{"taskId": 123}`), + ) + var logged bytes.Buffer + previous := log.Writer() + log.SetOutput(&logged) + defer log.SetOutput(previous) + + out, err := cmd.Execute("baremetal", "reinstall", "fakeBaremetal", "--os", "debian12_64", "--dry-run") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains(`"operatingSystem": "debian12_64"`), + "the payload is still printed, once, as the message") + assert.Cmp(logged.String(), td.Not(td.Contains("Final parameters")), + "a dry run logs nothing: it sends nothing") +} + +// The positive control of the test above: a REAL run still logs what it is +// about to send. Without it, deleting the log line altogether would pass. +func (ms *MockSuite) TestBaremetalReinstallStillLogsTheParametersItSends(assert, require *td.T) { + httpmock.RegisterResponder("POST", "https://eu.api.ovh.com/v1/dedicated/server/fakeBaremetal/reinstall", + httpmock.NewStringResponder(200, `{"taskId": 123}`), + ) + var logged bytes.Buffer + previous := log.Writer() + log.SetOutput(&logged) + defer log.SetOutput(previous) + + _, err := cmd.Execute("baremetal", "reinstall", "fakeBaremetal", "--os", "debian12_64", "--yes") + + require.CmpNoError(err) + assert.Cmp(logged.String(), td.Contains("Final parameters")) + assert.Cmp(logged.String(), td.Contains(`"operatingSystem": "debian12_64"`)) +} From 4e51d8a3258cda2fd66f82fe720b2136cacd080f Mon Sep 17 00:00:00 2001 From: Denis Date: Mon, 24 Aug 2026 12:17:08 +0200 Subject: [PATCH 10/10] chore(doc): regenerate after bringing in the parent branch Signed-off-by: Denis --- doc/ovhcloud.md | 125 ++++++++--------------------------------- doc/ovhcloud_logout.md | 46 +++++++++++++++ 2 files changed, 69 insertions(+), 102 deletions(-) create mode 100644 doc/ovhcloud_logout.md diff --git a/doc/ovhcloud.md b/doc/ovhcloud.md index 7d79d6150..acad5f7fc 100644 --- a/doc/ovhcloud.md +++ b/doc/ovhcloud.md @@ -1,90 +1,36 @@ -# OVHcloud CLI (`ovhcloud`) Documentation +## ovhcloud ---- +CLI to manage your OVHcloud services -## Overview +### Options -`ovhcloud` is a single, unified command‑line interface for managing the full range of OVHcloud products and account resources directly from your terminal. Whether you need to automate provisioning, perform quick look‑ups, or integrate OVHcloud operations into CI/CD pipelines, `ovhcloud` offers fine‑grained commands and consistent output formats (table, JSON, YAML, or custom gval expressions). - ---- - -## Quick Start - -```bash -# Display the top‑level help -ovhcloud --help - -# Log in and create API credentials (interactive) -ovhcloud login - -# List your VPS instances as JSON -ohvcloud vps list -o json -``` - -Check out the [authentication page](authentication.md) for further information about the authentication means. - -You can manage multiple OVHcloud accounts using [profiles](profiles.md). Create a profile with `ovhcloud login --profile `, switch between them with `ovhcloud config profile switch `, or use `--profile ` on any command. - -### Generate Shell Completion - -```bash -# Bash -eval "$(./ovhcloud completion bash)" -# Zsh -eval "$(./ovhcloud completion zsh)" -# Fish -./ovhcloud completion fish | source -# PowerShell -./ovhcloud completion powershell | Out-String | Invoke-Expression ``` - -Add the appropriate line to your shell’s startup file (`~/.bashrc`, `~/.zshrc`, etc.) to enable persistent autocompletion. - ---- - -## Global Usage - -```text -ovhcloud [command] [flags] + -d, --debug Activate debug mode (will log all HTTP requests details) + -h, --help help for ovhcloud + -e, --ignore-errors Ignore errors in API calls when it is not fatal to the execution + -o, --output string Output format: json, yaml, interactive, or a custom format expression (using https://github.com/PaesslerAG/gval syntax) + Examples: + --output json + --output yaml + --output interactive + --output 'id' (to extract a single field) + --output 'nested.field.subfield' (to extract a nested field) + --output '[id, "name"]' (to extract multiple fields as an array) + --output '{"newKey": oldKey, "otherKey": nested.field}' (to extract and rename fields in an object) + --output 'name+","+type' (to extract and concatenate fields in a string) + --output '(nbFieldA + nbFieldB) * 10' (to compute values from numeric fields) + --profile string Use a specific profile from the configuration file ``` -### Global Flags - -| Flag | Description | -| ------------------ | ---------------------------------------------------- | -| `--debug` | Activate debug mode (logs all HTTP‑request details). | -| `--ignore-errors` | Ignore errors of API calls made when listing items. | -| `--filter ` | Filter lists output with a [gval] expression. | -| `-h`, `--help` | Display help for `ovhcloud` or a specific command. | -| `-o interactive` | Produce interactive (prompt‑based) output. | -| `-o json` | Output data in JSON format. | -| `-o yaml` | Output data in YAML format. | -| `-o ` | Format output with a [gval] expression. | - -[gval]: https://github.com/PaesslerAG/gval - -#### Filtering examples - -- Strict string equality: `--filter 'name=="something"'` -- String regexp comparison: `--filter 'name=~"something"'` -- Number comparison: `--filter 'bootId > 1'` - -#### Formatting example - -- Extract only one field: `-o 'ip'` -- Extract an object: `-o '{name: ip}'` - ---- - -## Command Reference - -Below is the full list of primary sub‑commands available at the time of writing. Each can be explored in depth with `ovhcloud --help`. +### SEE ALSO * [ovhcloud account](ovhcloud_account.md) - Manage your account * [ovhcloud alldom](ovhcloud_alldom.md) - Retrieve information and manage your AllDom services * [ovhcloud baremetal](ovhcloud_baremetal.md) - Retrieve information and manage your Bare Metal services +* [ovhcloud browser](ovhcloud_browser.md) - Launch a TUI for the OVHcloud Manager - Public Cloud universe only [EXPERIMENTAL] * [ovhcloud cdn-dedicated](ovhcloud_cdn-dedicated.md) - Retrieve information and manage your dedicated CDN services * [ovhcloud cloud](ovhcloud_cloud.md) - Manage your projects and services in the Public Cloud universe (MKS, MPR, MRS, Object Storage...) +* [ovhcloud completion](ovhcloud_completion.md) - Generate shell completion scripts * [ovhcloud config](ovhcloud_config.md) - Manage your CLI configuration * [ovhcloud dedicated-ceph](ovhcloud_dedicated-ceph.md) - Retrieve information and manage your Dedicated Ceph services * [ovhcloud dedicated-cloud](ovhcloud_dedicated-cloud.md) - Retrieve information and manage your DedicatedCloud services @@ -102,6 +48,7 @@ Below is the full list of primary sub‑commands available at the time of writin * [ovhcloud ldp](ovhcloud_ldp.md) - Retrieve information and manage your LDP (Logs Data Platform) services * [ovhcloud location](ovhcloud_location.md) - Retrieve information and manage your Location services * [ovhcloud login](ovhcloud_login.md) - Login to your OVHcloud account to create API credentials +* [ovhcloud logout](ovhcloud_logout.md) - Revoke your API credentials and remove them from the configuration * [ovhcloud nutanix](ovhcloud_nutanix.md) - Retrieve information and manage your Nutanix services * [ovhcloud okms](ovhcloud_okms.md) - Retrieve information and manage your OKMS (Key Management Services) * [ovhcloud overthebox](ovhcloud_overthebox.md) - Retrieve information and manage your OverTheBox services @@ -113,6 +60,7 @@ Below is the full list of primary sub‑commands available at the time of writin * [ovhcloud storage-netapp](ovhcloud_storage-netapp.md) - Retrieve information and manage your Storage NetApp services * [ovhcloud support-tickets](ovhcloud_support-tickets.md) - Retrieve information and manage your support tickets * [ovhcloud telephony](ovhcloud_telephony.md) - Retrieve information and manage your Telephony services +* [ovhcloud upgrade](ovhcloud_upgrade.md) - Upgrade OVHcloud CLI to the latest version * [ovhcloud veeamcloudconnect](ovhcloud_veeamcloudconnect.md) - Retrieve information and manage your VeeamCloudConnect services * [ovhcloud veeamenterprise](ovhcloud_veeamenterprise.md) - Retrieve information and manage your VeeamEnterprise services * [ovhcloud version](ovhcloud_version.md) - Get OVHcloud CLI version @@ -124,30 +72,3 @@ Below is the full list of primary sub‑commands available at the time of writin * [ovhcloud webhosting](ovhcloud_webhosting.md) - Retrieve information and manage your WebHosting services * [ovhcloud xdsl](ovhcloud_xdsl.md) - Retrieve information and manage your XDSL services -> **Tip**  Use `-o json`, `-o yaml`, or `-o ` with a gval expression to integrate `ovhcloud` into scripts and automation pipelines. - ---- - -## Examples - -| Task | Command | -| ------------------------------------- | ---------------------------------------------- | -| Log in and save credentials | `ovhcloud login` | -| List VPS instances (tabular) | `ovhcloud vps list` | -| Fetch details of a single VPS in JSON | `ovhcloud vps get -o json` | -| Reinstall a baremetal interactively | `ovhcloud baremetal reinstall --editor` | - ---- - -## Troubleshooting - -* **Verbose output** — Use `--debug` to inspect raw API calls and responses. -* **Authentication issues** — Run `ovhcloud login` again to regenerate valid API keys. -* **Rate limits** — OVHcloud APIs impose rate limits; plan retries or exponential backoff in scripts. - ---- - -## Further Reading - -* OVHcloud API reference: [https://eu.api.ovh.com/console](https://eu.api.ovh.com/console) -* OVHcloud community guides and tutorials. diff --git a/doc/ovhcloud_logout.md b/doc/ovhcloud_logout.md new file mode 100644 index 000000000..c78e5c513 --- /dev/null +++ b/doc/ovhcloud_logout.md @@ -0,0 +1,46 @@ +## ovhcloud logout + +Revoke your API credentials and remove them from the configuration + +``` +ovhcloud logout [flags] +``` + +### Examples + +``` +ovhcloud logout +ovhcloud logout --yes +ovhcloud logout --profile work +``` + +### Options + +``` + -h, --help help for logout + -y, --yes Do not ask for confirmation +``` + +### Options inherited from parent commands + +``` + -d, --debug Activate debug mode (will log all HTTP requests details) + -e, --ignore-errors Ignore errors in API calls when it is not fatal to the execution + -o, --output string Output format: json, yaml, interactive, or a custom format expression (using https://github.com/PaesslerAG/gval syntax) + Examples: + --output json + --output yaml + --output interactive + --output 'id' (to extract a single field) + --output 'nested.field.subfield' (to extract a nested field) + --output '[id, "name"]' (to extract multiple fields as an array) + --output '{"newKey": oldKey, "otherKey": nested.field}' (to extract and rename fields in an object) + --output 'name+","+type' (to extract and concatenate fields in a string) + --output '(nbFieldA + nbFieldB) * 10' (to compute values from numeric fields) + --profile string Use a specific profile from the configuration file +``` + +### SEE ALSO + +* [ovhcloud](ovhcloud.md) - CLI to manage your OVHcloud services +