From d2de4c3a506c4c92ba452e933e382149dc8d8748 Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Tue, 18 Aug 2026 17:02:04 +0200 Subject: [PATCH 01/11] 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/11] 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 3176e6aa68c0215ff512c9cc160b9b836071a3c4 Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Thu, 20 Aug 2026 01:43:01 +0200 Subject: [PATCH 03/11] feat(baremetal): send a server's logs somewhere, from the terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven v2 routes subscribe a dedicated server's logs to a Log Data Platform stream, and none of them was reachable here. Not skipped — invisible: the repository could only fetch v1 schemas, so the surface never appeared in any inventory of what was missing. ovhcloud baremetal logs kinds ovhcloud baremetal logs url ovhcloud baremetal logs subscription list|get [id] ovhcloud baremetal logs subscribe --stream [--wait] ovhcloud baremetal logs unsubscribe [--wait] Three things measured against the real account changed the design before a line was written. It is not a tail, so it is not called one. POST .../log/url returns a signed get.logs.ovh.com/search address into the Log Data Platform search interface, valid for exactly thirty minutes. The command prints the link, says when it dies, and says that it carries its own authorisation. There is one kind of log — "install" — and --kind is optional in both directions. Making somebody type the only possible value is friction; putting "install" in Go as a default would keep reaching that one kind on the day a second appears. The server is asked instead: one kind is taken, several are refused with their names, none is an answer rather than an error. The subscription carries its own LDP service in serviceName, and so does the POST response, so --wait is told where to poll. The stream resolution is the command. An operator has a server name and wants "my logs, over there"; they do not have a stream UUID, and this account holds 59 streams across 25 services. --stream therefore takes a title or an identifier, resolved the way `vrack attach` resolves an interface. A title carried by more than one stream is refused with both identifiers and their services — three titles on this account really are, and the account holds both "Test" and "test", so the match is case-sensitive. The sweep is 85 requests and 2.15 seconds, and none of it is cached: a stale identifier would send a machine's logs to the wrong place. Only the completion suggestions are. --wait crosses both catalogues. The change is made on /v2 and no v2 route can follow it; it is followed on /v1/dbaas/logs/{service}/operation/{id}, whose state vocabulary is a sixth one in this repository and shares nothing with the five already here. And SUCCESS is not the subscription existing: the operation carries subscriptionId, so the wait reads the state back, and the removal checks the other way round. Two traps avoided by name. The --kind filter stays on the collection call: the expansion builds one URL per subscription as path + "/%s", so a query carried over would produce ".../log/subscription?kind=install/", a route that does not exist for a filter that would have looked like it worked. And the dry run shows the body, because what an operator checks before agreeing is which stream and which kind, and neither is in the URL. Verified on ns3118333.ip-51-68-100.eu against a stream its owner had named "TO REMOVE 1": subscribe --wait in 7s, read back outside the CLI, unsubscribe --wait in 6s, final state read outside the CLI identical to the start, and the same removal refused a second time. 20 tests (6 unit, 14 cobra), 12 sabotages, 12 red. Signed-off-by: Denis Hamon --- doc/ovhcloud_baremetal.md | 1 + doc/ovhcloud_baremetal_logs.md | 38 + doc/ovhcloud_baremetal_logs_kinds.md | 37 + doc/ovhcloud_baremetal_logs_subscribe.md | 48 + doc/ovhcloud_baremetal_logs_subscription.md | 35 + ...vhcloud_baremetal_logs_subscription_get.md | 37 + ...hcloud_baremetal_logs_subscription_list.md | 45 + doc/ovhcloud_baremetal_logs_unsubscribe.md | 40 + doc/ovhcloud_baremetal_logs_url.md | 45 + internal/assets/api-schemas/baremetal_v2.json | 1023 +++++++++++++++++ internal/cmd/baremetal.go | 104 ++ internal/cmd/baremetal_logs_test.go | 267 +++++ internal/services/baremetal/logs.go | 506 ++++++++ internal/services/baremetal/logs_stream.go | 300 +++++ internal/services/baremetal/logs_test.go | 131 +++ 15 files changed, 2657 insertions(+) create mode 100644 doc/ovhcloud_baremetal_logs.md create mode 100644 doc/ovhcloud_baremetal_logs_kinds.md create mode 100644 doc/ovhcloud_baremetal_logs_subscribe.md create mode 100644 doc/ovhcloud_baremetal_logs_subscription.md create mode 100644 doc/ovhcloud_baremetal_logs_subscription_get.md create mode 100644 doc/ovhcloud_baremetal_logs_subscription_list.md create mode 100644 doc/ovhcloud_baremetal_logs_unsubscribe.md create mode 100644 doc/ovhcloud_baremetal_logs_url.md create mode 100644 internal/assets/api-schemas/baremetal_v2.json create mode 100644 internal/cmd/baremetal_logs_test.go create mode 100644 internal/services/baremetal/logs.go create mode 100644 internal/services/baremetal/logs_stream.go create mode 100644 internal/services/baremetal/logs_test.go diff --git a/doc/ovhcloud_baremetal.md b/doc/ovhcloud_baremetal.md index 4d545cc5d..42710243c 100644 --- a/doc/ovhcloud_baremetal.md +++ b/doc/ovhcloud_baremetal.md @@ -46,6 +46,7 @@ Retrieve information and manage your Bare Metal services * [ovhcloud baremetal list-partition-schemes](ovhcloud_baremetal_list-partition-schemes.md) - List the partition schemes an OS template allows on this baremetal * [ovhcloud baremetal list-secrets](ovhcloud_baremetal_list-secrets.md) - Retrieve secrets to connect to the server * [ovhcloud baremetal list-tasks](ovhcloud_baremetal_list-tasks.md) - Retrieve tasks of the given baremetal +* [ovhcloud baremetal logs](ovhcloud_baremetal_logs.md) - Read the logs of a dedicated server, and send them to a stream * [ovhcloud baremetal power](ovhcloud_baremetal_power.md) - Power the given baremetal off and on * [ovhcloud baremetal raid-profile](ovhcloud_baremetal_raid-profile.md) - Show the hardware RAID controllers of this baremetal, if it has any * [ovhcloud baremetal reboot](ovhcloud_baremetal_reboot.md) - Reboot the given baremetal diff --git a/doc/ovhcloud_baremetal_logs.md b/doc/ovhcloud_baremetal_logs.md new file mode 100644 index 000000000..b9ced404b --- /dev/null +++ b/doc/ovhcloud_baremetal_logs.md @@ -0,0 +1,38 @@ +## ovhcloud baremetal logs + +Read the logs of a dedicated server, and send them to a stream + +### Options + +``` + -h, --help help for logs +``` + +### 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 baremetal](ovhcloud_baremetal.md) - Retrieve information and manage your Bare Metal services +* [ovhcloud baremetal logs kinds](ovhcloud_baremetal_logs_kinds.md) - List the kinds of log this server can send +* [ovhcloud baremetal logs subscribe](ovhcloud_baremetal_logs_subscribe.md) - Send the logs of this server to a Log Data Platform stream +* [ovhcloud baremetal logs subscription](ovhcloud_baremetal_logs_subscription.md) - Show where the logs of this server are sent +* [ovhcloud baremetal logs unsubscribe](ovhcloud_baremetal_logs_unsubscribe.md) - Stop sending the logs of this server to a stream +* [ovhcloud baremetal logs url](ovhcloud_baremetal_logs_url.md) - Get a temporary link to read the logs of this server + diff --git a/doc/ovhcloud_baremetal_logs_kinds.md b/doc/ovhcloud_baremetal_logs_kinds.md new file mode 100644 index 000000000..042290846 --- /dev/null +++ b/doc/ovhcloud_baremetal_logs_kinds.md @@ -0,0 +1,37 @@ +## ovhcloud baremetal logs kinds + +List the kinds of log this server can send + +``` +ovhcloud baremetal logs kinds [flags] +``` + +### Options + +``` + -h, --help help for kinds +``` + +### 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 baremetal logs](ovhcloud_baremetal_logs.md) - Read the logs of a dedicated server, and send them to a stream + diff --git a/doc/ovhcloud_baremetal_logs_subscribe.md b/doc/ovhcloud_baremetal_logs_subscribe.md new file mode 100644 index 000000000..37693bd55 --- /dev/null +++ b/doc/ovhcloud_baremetal_logs_subscribe.md @@ -0,0 +1,48 @@ +## ovhcloud baremetal logs subscribe + +Send the logs of this server to a Log Data Platform stream + +### Synopsis + +Send the logs of this server to a Log Data Platform stream. + +--stream takes the title of a stream or its identifier. A title is resolved across every Log Data Platform service on the account, and a title carried by more than one stream is refused rather than guessed. + +``` +ovhcloud baremetal logs subscribe [flags] +``` + +### Options + +``` + --dry-run Print the call that would be made without making it + -h, --help help for subscribe + --kind string Kind of log to work on (default: the only one the server offers) + --stream string Title or identifier of the stream to send the logs to + --wait Wait until the subscription actually exists before exiting + -y, --yes Skip the confirmation prompt (required for unattended runs) +``` + +### 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 baremetal logs](ovhcloud_baremetal_logs.md) - Read the logs of a dedicated server, and send them to a stream + diff --git a/doc/ovhcloud_baremetal_logs_subscription.md b/doc/ovhcloud_baremetal_logs_subscription.md new file mode 100644 index 000000000..7e3bae140 --- /dev/null +++ b/doc/ovhcloud_baremetal_logs_subscription.md @@ -0,0 +1,35 @@ +## ovhcloud baremetal logs subscription + +Show where the logs of this server are sent + +### Options + +``` + -h, --help help for subscription +``` + +### 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 baremetal logs](ovhcloud_baremetal_logs.md) - Read the logs of a dedicated server, and send them to a stream +* [ovhcloud baremetal logs subscription get](ovhcloud_baremetal_logs_subscription_get.md) - Show one log subscription of this server +* [ovhcloud baremetal logs subscription list](ovhcloud_baremetal_logs_subscription_list.md) - List the log subscriptions of this server + diff --git a/doc/ovhcloud_baremetal_logs_subscription_get.md b/doc/ovhcloud_baremetal_logs_subscription_get.md new file mode 100644 index 000000000..72c9e0a4c --- /dev/null +++ b/doc/ovhcloud_baremetal_logs_subscription_get.md @@ -0,0 +1,37 @@ +## ovhcloud baremetal logs subscription get + +Show one log subscription of this server + +``` +ovhcloud baremetal logs subscription get [flags] +``` + +### Options + +``` + -h, --help help for get +``` + +### 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 baremetal logs subscription](ovhcloud_baremetal_logs_subscription.md) - Show where the logs of this server are sent + diff --git a/doc/ovhcloud_baremetal_logs_subscription_list.md b/doc/ovhcloud_baremetal_logs_subscription_list.md new file mode 100644 index 000000000..a99877375 --- /dev/null +++ b/doc/ovhcloud_baremetal_logs_subscription_list.md @@ -0,0 +1,45 @@ +## ovhcloud baremetal logs subscription list + +List the log subscriptions of this server + +``` +ovhcloud baremetal logs subscription list [flags] +``` + +### Options + +``` + --filter stringArray Filter results by any property using https://github.com/PaesslerAG/gval syntax + Examples: + --filter 'state=="running"' + --filter 'name=~"^my.*"' + --filter 'nested.property.subproperty>10' + --filter 'startDate>="2023-12-01"' + --filter 'name=~"something" && nbField>10' + -h, --help help for list + --kind string Kind of log to work on (default: the only one the server offers) +``` + +### 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 baremetal logs subscription](ovhcloud_baremetal_logs_subscription.md) - Show where the logs of this server are sent + diff --git a/doc/ovhcloud_baremetal_logs_unsubscribe.md b/doc/ovhcloud_baremetal_logs_unsubscribe.md new file mode 100644 index 000000000..6ae6a555f --- /dev/null +++ b/doc/ovhcloud_baremetal_logs_unsubscribe.md @@ -0,0 +1,40 @@ +## ovhcloud baremetal logs unsubscribe + +Stop sending the logs of this server to a stream + +``` +ovhcloud baremetal logs unsubscribe [flags] +``` + +### Options + +``` + --dry-run Print the call that would be made without making it + -h, --help help for unsubscribe + --wait Wait until the subscription is actually gone before exiting + -y, --yes Skip the confirmation prompt (required for unattended runs) +``` + +### 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 baremetal logs](ovhcloud_baremetal_logs.md) - Read the logs of a dedicated server, and send them to a stream + diff --git a/doc/ovhcloud_baremetal_logs_url.md b/doc/ovhcloud_baremetal_logs_url.md new file mode 100644 index 000000000..70558d7c0 --- /dev/null +++ b/doc/ovhcloud_baremetal_logs_url.md @@ -0,0 +1,45 @@ +## ovhcloud baremetal logs url + +Get a temporary link to read the logs of this server + +### Synopsis + +Get a temporary link to read the logs of this server. + +The link opens the Log Data Platform search interface and carries its own authorisation, so anyone holding it can read these logs. It expires; the command says when. + +``` +ovhcloud baremetal logs url [flags] +``` + +### Options + +``` + --dry-run Print the call that would be made without making it + -h, --help help for url + --kind string Kind of log to work on (default: the only one the server offers) +``` + +### 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 baremetal logs](ovhcloud_baremetal_logs.md) - Read the logs of a dedicated server, and send them to a stream + diff --git a/internal/assets/api-schemas/baremetal_v2.json b/internal/assets/api-schemas/baremetal_v2.json new file mode 100644 index 000000000..f505311fd --- /dev/null +++ b/internal/assets/api-schemas/baremetal_v2.json @@ -0,0 +1,1023 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "OVHcloud API specification", + "description": "Specification for OVHcloud API", + "version": "1.0", + "contact": { + "name": "OVH", + "email": "api@ml.ovh.net" + } + }, + "servers": [ + { + "url": "https://eu.api.ovh.com/v2" + } + ], + "components": { + "schemas": { + "any": { + "description": "Can be anything (text, int, bool, json, html, etc.)", + "example": "This is a content of any type !" + }, + "dbaas.logs.LogKind": { + "type": "object", + "description": "Log kind", + "properties": { + "additionalReturnedFields": { + "type": "array", + "description": "List of additional log fields managed in this log kind", + "items": { + "type": "string" + }, + "readOnly": true + }, + "createdAt": { + "type": "string", + "description": "Creation date of the log kind", + "format": "date-time", + "readOnly": true + }, + "displayName": { + "type": "string", + "description": "Log kind display name", + "readOnly": true + }, + "kindId": { + "type": "string", + "description": "Log kind ID", + "format": "uuid", + "readOnly": true + }, + "name": { + "type": "string", + "description": "Log kind name", + "readOnly": true + }, + "updatedAt": { + "type": "string", + "description": "Last update date of the log kind", + "format": "date-time", + "readOnly": true + } + } + }, + "dbaas.logs.LogSubscription": { + "type": "object", + "description": "Log subscription", + "properties": { + "createdAt": { + "type": "string", + "description": "Creation date of the subscription", + "format": "date-time", + "readOnly": true + }, + "kind": { + "type": "string", + "description": "Log kind name of this subscription", + "readOnly": true + }, + "resource": { + "allOf": [ + { + "$ref": "#/components/schemas/dbaas.logs.LogSubscriptionResource" + } + ], + "description": "Subscribed resource, where the logs come from", + "readOnly": true + }, + "serviceName": { + "type": "string", + "description": "Name of the destination log service", + "readOnly": true + }, + "streamId": { + "type": "string", + "description": "Id of the destination log stream", + "readOnly": true + }, + "subscriptionId": { + "type": "string", + "description": "Subscription ID", + "format": "uuid", + "readOnly": true + }, + "updatedAt": { + "type": "string", + "description": "Last update date of the subscription", + "format": "date-time", + "readOnly": true + } + } + }, + "dbaas.logs.LogSubscriptionCreation": { + "type": "object", + "description": "Log subscription creation payload", + "properties": { + "kind": { + "type": "string", + "description": "Log kind name to subscribe to" + }, + "streamId": { + "type": "string", + "description": "Customer log stream ID", + "format": "uuid" + } + }, + "required": [ + "kind", + "streamId" + ] + }, + "dbaas.logs.LogSubscriptionResource": { + "type": "object", + "description": "Log subscription resource", + "properties": { + "name": { + "type": "string", + "description": "Name of subscribed resource", + "readOnly": true + }, + "type": { + "type": "string", + "description": "Type of subscribed resource", + "readOnly": true + } + } + }, + "dbaas.logs.LogSubscriptionResponse": { + "type": "object", + "description": "Asynchronous operation after subscribing or unsubscribing to a resource logs", + "properties": { + "operationId": { + "type": "string", + "description": "Identifier of the operation", + "format": "uuid", + "readOnly": true + }, + "serviceName": { + "type": "string", + "description": "Operation owner's service name", + "readOnly": true + } + } + }, + "dbaas.logs.LogUrlCreation": { + "type": "object", + "description": "Log temporary URL creation payload", + "properties": { + "kind": { + "type": "string", + "description": "Log kind name" + } + }, + "required": [ + "kind" + ] + }, + "dbaas.logs.TemporaryLogsLink": { + "type": "object", + "description": "Temporary url information", + "properties": { + "expirationDate": { + "type": "string", + "description": "Temporary url expiration date", + "format": "date-time", + "readOnly": true + }, + "url": { + "type": "string", + "description": "Temporary url", + "readOnly": true + } + } + }, + "dedicated.server": { + "type": "object", + "description": "Dedicated server resource", + "properties": { + "id": { + "type": "string", + "description": "Service name of the dedicated server resource", + "readOnly": true + } + } + }, + "dedicated.serverWithIAM": { + "type": "object", + "description": "Dedicated server resource", + "properties": { + "iam": { + "allOf": [ + { + "$ref": "#/components/schemas/iam.ResourceMetadata" + } + ], + "description": "IAM resource metadata", + "nullable": true, + "readOnly": true + }, + "id": { + "type": "string", + "description": "Service name of the dedicated server resource", + "readOnly": true + } + } + }, + "duration": { + "type": "string", + "description": "Duration (e.g., P1Y2M3DT4H6M9S)", + "example": "P1Y2M3DT4H6M9S" + }, + "email": { + "type": "string", + "description": "Email address (e.g., api@ml.ovh.net)", + "format": "email", + "example": "api@ml.ovh.net" + }, + "iam.ResourceMetadata": { + "type": "object", + "description": "IAM resource metadata embedded in services models", + "properties": { + "displayName": { + "type": "string", + "description": "Resource display name", + "nullable": true, + "readOnly": true + }, + "id": { + "type": "string", + "description": "Unique identifier of the resource", + "format": "uuid", + "readOnly": true + }, + "state": { + "allOf": [ + { + "$ref": "#/components/schemas/iam.ResourceMetadata.StateEnum" + } + ], + "description": "Resource state", + "nullable": true, + "readOnly": true + }, + "tags": { + "type": "object", + "description": "Resource tags. Tags that were internally computed are prefixed with ovh:", + "nullable": true, + "additionalProperties": { + "type": "string" + }, + "readOnly": true + }, + "urn": { + "type": "string", + "description": "Unique resource name used in policies", + "readOnly": true + } + } + }, + "iam.ResourceMetadata.StateEnum": { + "type": "string", + "description": "Resource state", + "enum": [ + "EXPIRED", + "IN_CREATION", + "OK", + "SUSPENDED" + ] + }, + "iam.resource.TagFilter": { + "type": "object", + "description": "Resource tag filter", + "properties": { + "operator": { + "allOf": [ + { + "$ref": "#/components/schemas/iam.resource.TagFilter.OperatorEnum" + } + ], + "description": "Operator to use in order to filter on the value (defaults to 'EQ')", + "nullable": true, + "readOnly": true + }, + "value": { + "type": "string", + "description": "Value to use in order to filter tags", + "readOnly": true + } + } + }, + "iam.resource.TagFilter.OperatorEnum": { + "type": "string", + "description": "Operator that can be used in order to filter resources tags", + "enum": [ + "EQ", + "EXISTS", + "ILIKE", + "LIKE", + "NEQ", + "NEXISTS" + ] + }, + "internationalPhoneNumber": { + "type": "string", + "description": "International phone number", + "format": "phone-number" + }, + "ip": { + "type": "string", + "description": "IP address (e.g., 192.0.2.0)", + "format": "ipv4", + "example": "192.0.2.0" + }, + "ipBlock": { + "type": "string", + "description": "IP (v4 or v6) CIDR notation (e.g., 192.0.2.0/24)", + "format": "ip-block", + "example": "192.0.2.0/24" + }, + "ipInterface": { + "type": "string", + "description": "IP address (e.g., 192.0.2.0)", + "format": "ipv4", + "example": "192.0.2.0" + }, + "ipv4": { + "type": "string", + "description": "IPv4 address (e.g., 192.0.2.0)", + "format": "ipv4", + "example": "192.0.2.0" + }, + "ipv4Block": { + "type": "string", + "description": "IPv4 CIDR notation (e.g., 192.0.2.0/24)", + "format": "ipv4-block", + "example": "192.0.2.0/24" + }, + "ipv6": { + "type": "string", + "description": "IPv6 address (e.g., 2001:41d0:1:1994::1)", + "format": "ipv6", + "example": "2001:41d0:1:1994::1" + }, + "ipv6Block": { + "type": "string", + "description": "IPv6 CIDR notation (e.g., 2001:41d0::/128)", + "format": "ipv6-block", + "example": "2001:41d0::/128" + }, + "macAddress": { + "type": "string", + "description": "MAC address (e.g., 00:00:5e:00:53:01)", + "format": "mac-address", + "example": "00:00:5e:00:53:01" + }, + "phoneNumber": { + "type": "string", + "description": "Phone number", + "format": "phone-number" + }, + "time": { + "type": "string", + "description": "Time (e.g., 15:04:05)", + "format": "time", + "example": "15:04:05" + } + }, + "securitySchemes": { + "oAuth2AuthCode": { + "type": "oauth2", + "description": "Oauth2", + "x-client-id": "1bb9c7df371741c0", + "x-client-secret": "a5b4de870aca620d10fbf63cd18d205b", + "flows": { + "authorizationCode": { + "authorizationUrl": "https://auth.eu.ovhcloud.com/oauth2/authorize", + "tokenUrl": "https://auth.eu.ovhcloud.com/oauth2/token", + "scopes": { + "account/all": "Manage your account", + "all": "Manage your whole account and all your services", + "services/all": "Manage your services lifecycle and billing" + } + } + } + } + } + }, + "paths": { + "/dedicated/server": { + "get": { + "summary": "List dedicated server resources", + "description": "Get the list of all my dedicated server resources", + "security": [ + { + "oAuth2AuthCode": [] + } + ], + "parameters": [ + { + "in": "query", + "name": "iamTags", + "description": "Filter resources on IAM tags", + "schema": { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "$ref": "#/components/schemas/iam.resource.TagFilter" + } + } + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "$ref": "#/components/schemas/dedicated.serverWithIAM" + } + } + } + } + } + }, + "x-developer-hub-navigation": [ + [ + "Bare Metal Cloud", + "Dedicated Servers" + ] + ], + "x-badges": [ + { + "color": "blue", + "label": "Alpha version" + } + ], + "x-iam-actions": [ + { + "name": "dedicatedServer:apiovh:get", + "required": true + } + ], + "x-expanded-response": "DedicatedServerWithIAM" + } + }, + "/dedicated/server/{serviceName}": { + "get": { + "summary": "Get dedicated server resource", + "description": "Get the details of the dedicated server resource", + "security": [ + { + "oAuth2AuthCode": [] + } + ], + "parameters": [ + { + "in": "path", + "name": "serviceName", + "description": "Service name", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/dedicated.serverWithIAM" + } + } + } + }, + "404": { + "description": "Error 404 response", + "content": { + "application/json": { + "schema": { + "type": "object", + "description": "Common error model", + "properties": { + "class": { + "type": "string", + "description": "Class of the error" + }, + "message": { + "type": "string", + "description": "Error message" + } + } + }, + "examples": { + "Client::NotFound::DedicatedServer": { + "value": { + "class": "Client::NotFound::DedicatedServer", + "message": "Dedicated server resource {{.id}} not found" + } + }, + "Client::NotFound::DedicatedServerMultiple": { + "value": { + "class": "Client::NotFound::DedicatedServerMultiple", + "message": "At least one of the dedicated server resource(s) requested was not found" + } + } + } + } + } + } + }, + "x-developer-hub-navigation": [ + [ + "Bare Metal Cloud", + "Dedicated Servers" + ] + ], + "x-badges": [ + { + "color": "blue", + "label": "Alpha version" + } + ], + "x-iam-actions": [ + { + "name": "dedicatedServer:apiovh:get", + "required": true + } + ] + } + }, + "/dedicated/server/{serviceName}/log/kind": { + "get": { + "summary": "List available log kinds", + "description": "Get the list of log kinds available for subscription or live tail on a dedicated server resource", + "security": [ + { + "oAuth2AuthCode": [] + } + ], + "parameters": [ + { + "in": "header", + "name": "X-Pagination-Cursor", + "description": "Pagination cursor", + "schema": { + "type": "string" + } + }, + { + "in": "header", + "name": "X-Pagination-Size", + "description": "Pagination size", + "schema": { + "type": "integer" + } + }, + { + "in": "path", + "name": "serviceName", + "description": "Service name", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } + }, + "x-developer-hub-navigation": [ + [ + "Bare Metal Cloud", + "Dedicated Servers" + ] + ], + "x-badges": [ + { + "color": "blue", + "label": "Alpha version" + } + ], + "x-iam-actions": [ + { + "name": "dedicatedServer:apiovh:log/kind/get", + "required": true + } + ], + "x-expanded-response": "DbaasLogsLogKind" + } + }, + "/dedicated/server/{serviceName}/log/kind/{name}": { + "get": { + "summary": "Get a log kind", + "description": "Get the details of a specific log kind on a dedicated server resource", + "security": [ + { + "oAuth2AuthCode": [] + } + ], + "parameters": [ + { + "in": "path", + "name": "name", + "description": "Name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "in": "path", + "name": "serviceName", + "description": "Service name", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/dbaas.logs.LogKind" + } + } + } + } + }, + "x-developer-hub-navigation": [ + [ + "Bare Metal Cloud", + "Dedicated Servers" + ] + ], + "x-badges": [ + { + "color": "blue", + "label": "Alpha version" + } + ], + "x-iam-actions": [ + { + "name": "dedicatedServer:apiovh:log/kind/get", + "required": true + } + ], + "x-response-identifier": "name" + } + }, + "/dedicated/server/{serviceName}/log/subscription": { + "get": { + "summary": "List log forwarding subscriptions", + "description": "List the identifiers of all log forwarding subscriptions of a dedicated server resource", + "security": [ + { + "oAuth2AuthCode": [] + } + ], + "parameters": [ + { + "in": "header", + "name": "X-Pagination-Cursor", + "description": "Pagination cursor", + "schema": { + "type": "string" + } + }, + { + "in": "header", + "name": "X-Pagination-Size", + "description": "Pagination size", + "schema": { + "type": "integer" + } + }, + { + "in": "query", + "name": "kind", + "description": "Filter on a specific kind", + "schema": { + "type": "string" + } + }, + { + "in": "path", + "name": "serviceName", + "description": "Service name", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "type": "string", + "format": "uuid" + } + } + } + } + } + }, + "x-developer-hub-navigation": [ + [ + "Bare Metal Cloud", + "Dedicated Servers" + ] + ], + "x-badges": [ + { + "color": "blue", + "label": "Alpha version" + } + ], + "x-iam-actions": [ + { + "name": "dedicatedServer:apiovh:log/subscription/get", + "required": true + } + ], + "x-expanded-response": "DbaasLogsLogSubscription" + }, + "post": { + "summary": "Create a log forwarding subscription", + "description": "Create a persistent forwarding pipeline so that logs of the given kind are continuously written to the customer destination stream.", + "security": [ + { + "oAuth2AuthCode": [] + } + ], + "parameters": [ + { + "in": "path", + "name": "serviceName", + "description": "Service name", + "required": true, + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/dbaas.logs.LogSubscriptionCreation" + } + } + } + }, + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/dbaas.logs.LogSubscriptionResponse" + } + } + } + } + }, + "x-developer-hub-navigation": [ + [ + "Bare Metal Cloud", + "Dedicated Servers" + ] + ], + "x-badges": [ + { + "color": "blue", + "label": "Alpha version" + } + ], + "x-iam-actions": [ + { + "name": "dedicatedServer:apiovh:log/subscription", + "required": true + }, + { + "name": "ldp:apiovh:output/graylog/stream/forwardTo", + "description": "Required on the targeted LDP service", + "required": true + } + ] + } + }, + "/dedicated/server/{serviceName}/log/subscription/{subscriptionId}": { + "delete": { + "summary": "Delete a log forwarding subscription", + "description": "Tear down the persistent forwarding pipeline of a subscription.", + "security": [ + { + "oAuth2AuthCode": [] + } + ], + "parameters": [ + { + "in": "path", + "name": "serviceName", + "description": "Service name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "in": "path", + "name": "subscriptionId", + "description": "Subscription ID", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/dbaas.logs.LogSubscriptionResponse" + } + } + } + } + }, + "x-developer-hub-navigation": [ + [ + "Bare Metal Cloud", + "Dedicated Servers" + ] + ], + "x-badges": [ + { + "color": "blue", + "label": "Alpha version" + } + ], + "x-iam-actions": [ + { + "name": "dedicatedServer:apiovh:log/subscription/delete", + "required": true + } + ] + }, + "get": { + "summary": "Get a log forwarding subscription", + "description": "Get the details of a specific log forwarding subscription of a dedicated server resource", + "security": [ + { + "oAuth2AuthCode": [] + } + ], + "parameters": [ + { + "in": "path", + "name": "serviceName", + "description": "Service name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "in": "path", + "name": "subscriptionId", + "description": "Subscription ID", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/dbaas.logs.LogSubscription" + } + } + } + } + }, + "x-developer-hub-navigation": [ + [ + "Bare Metal Cloud", + "Dedicated Servers" + ] + ], + "x-badges": [ + { + "color": "blue", + "label": "Alpha version" + } + ], + "x-iam-actions": [ + { + "name": "dedicatedServer:apiovh:log/subscription/get", + "required": true + } + ], + "x-response-identifier": "subscriptionId" + } + }, + "/dedicated/server/{serviceName}/log/url": { + "post": { + "summary": "Generate a live tail URL", + "description": "Generate an ephemeral signed URL granting read-only real-time access to the dedicated server resource logs of the given kind. No persistent infrastructure is created.", + "security": [ + { + "oAuth2AuthCode": [] + } + ], + "parameters": [ + { + "in": "path", + "name": "serviceName", + "description": "Service name", + "required": true, + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/dbaas.logs.LogUrlCreation" + } + } + } + }, + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/dbaas.logs.TemporaryLogsLink" + } + } + } + } + }, + "x-developer-hub-navigation": [ + [ + "Bare Metal Cloud", + "Dedicated Servers" + ] + ], + "x-badges": [ + { + "color": "blue", + "label": "Alpha version" + } + ], + "x-iam-actions": [ + { + "name": "dedicatedServer:apiovh:log/url", + "required": true + } + ] + } + } + } +} diff --git a/internal/cmd/baremetal.go b/internal/cmd/baremetal.go index 7a1c511de..1c471f1db 100644 --- a/internal/cmd/baremetal.go +++ b/internal/cmd/baremetal.go @@ -808,9 +808,113 @@ sending. --dry-run prints the whole message instead of sending it.`, addConfirmationFlags(baremetalBackupCloudPasswordCmd, "Print the call that would be made without making it") baremetalBackupCloudCmd.AddCommand(baremetalBackupCloudPasswordCmd) + // Seven routes in the v2 catalogue send a server's logs to a Log Data + // Platform stream, and none of them was reachable: the CLI could not even + // fetch a schema for v2 until #262. The whole group is badged "Alpha + // version" upstream. + baremetalLogsCmd := &cobra.Command{ + Use: "logs", + Short: "Read the logs of a dedicated server, and send them to a stream", + } + baremetalCmd.AddCommand(baremetalLogsCmd) + + baremetalLogsCmd.AddCommand(&cobra.Command{ + Use: "kinds ", + Short: "List the kinds of log this server can send", + Args: cobra.ExactArgs(1), + ValidArgsFunction: completion.ServiceList("/v1/dedicated/server"), + Run: baremetal.ListBaremetalLogKinds, + }) + + baremetalLogsUrlCmd := &cobra.Command{ + Use: "url ", + Short: "Get a temporary link to read the logs of this server", + Long: "Get a temporary link to read the logs of this server.\n\n" + + "The link opens the Log Data Platform search interface and carries its own " + + "authorisation, so anyone holding it can read these logs. It expires; the command " + + "says when.", + Args: cobra.ExactArgs(1), + ValidArgsFunction: completion.ServiceList("/v1/dedicated/server"), + Run: baremetal.ShowBaremetalLogURL, + } + addLogKindFlag(baremetalLogsUrlCmd) + baremetalLogsUrlCmd.Flags().BoolVar(&flags.DryRun, "dry-run", + false, "Print the call that would be made without making it") + baremetalLogsCmd.AddCommand(baremetalLogsUrlCmd) + + baremetalLogsSubscriptionCmd := &cobra.Command{ + Use: "subscription", + Short: "Show where the logs of this server are sent", + } + baremetalLogsCmd.AddCommand(baremetalLogsSubscriptionCmd) + + baremetalLogsSubscriptionListCmd := withFilterFlag(&cobra.Command{ + Use: "list ", + Short: "List the log subscriptions of this server", + Args: cobra.ExactArgs(1), + ValidArgsFunction: completion.ServiceList("/v1/dedicated/server"), + Run: baremetal.ListBaremetalLogSubscriptions, + }) + addLogKindFlag(baremetalLogsSubscriptionListCmd) + baremetalLogsSubscriptionCmd.AddCommand(baremetalLogsSubscriptionListCmd) + + baremetalLogsSubscriptionCmd.AddCommand(&cobra.Command{ + Use: "get ", + Short: "Show one log subscription of this server", + Args: cobra.ExactArgs(2), + ValidArgsFunction: completion.ServiceList("/v1/dedicated/server"), + Run: baremetal.ShowBaremetalLogSubscription, + }) + + baremetalLogsSubscribeCmd := &cobra.Command{ + Use: "subscribe ", + Short: "Send the logs of this server to a Log Data Platform stream", + Long: "Send the logs of this server to a Log Data Platform stream.\n\n" + + "--stream takes the title of a stream or its identifier. A title is resolved " + + "across every Log Data Platform service on the account, and a title carried by " + + "more than one stream is refused rather than guessed.", + Args: cobra.ExactArgs(1), + ValidArgsFunction: completion.ServiceList("/v1/dedicated/server"), + Run: baremetal.SubscribeBaremetalLogs, + } + baremetalLogsSubscribeCmd.Flags().StringVar(&baremetal.LogStream, "stream", "", + "Title or identifier of the stream to send the logs to") + _ = baremetalLogsSubscribeCmd.MarkFlagRequired("stream") + _ = baremetalLogsSubscribeCmd.RegisterFlagCompletionFunc("stream", baremetal.CompleteLogStream) + addLogKindFlag(baremetalLogsSubscribeCmd) + baremetalLogsSubscribeCmd.Flags().BoolVar(&baremetal.LogWait, "wait", false, + "Wait until the subscription actually exists before exiting") + addConfirmationFlags(baremetalLogsSubscribeCmd, "Print the call that would be made without making it") + baremetalLogsCmd.AddCommand(baremetalLogsSubscribeCmd) + + baremetalLogsUnsubscribeCmd := &cobra.Command{ + Use: "unsubscribe ", + Short: "Stop sending the logs of this server to a stream", + Args: cobra.ExactArgs(2), + ValidArgsFunction: completion.ServiceList("/v1/dedicated/server"), + Run: baremetal.UnsubscribeBaremetalLogs, + } + baremetalLogsUnsubscribeCmd.Flags().BoolVar(&baremetal.LogWait, "wait", false, + "Wait until the subscription is actually gone before exiting") + addConfirmationFlags(baremetalLogsUnsubscribeCmd, "Print the call that would be made without making it") + baremetalLogsCmd.AddCommand(baremetalLogsUnsubscribeCmd) + rootCmd.AddCommand(baremetalCmd) } +// addLogKindFlag registers the kind of log a command works on. +// +// It is never required. Only one kind exists today, so a command that demanded +// it would make everybody type the only possible value; a command that +// defaulted to it in Go would still be reaching that one kind on the day a +// second appears. Left empty, the server is asked, and it is the answer that +// decides — one kind is taken, several are refused with their names. +func addLogKindFlag(cmd *cobra.Command) { + cmd.Flags().StringVar(&baremetal.LogKind, "kind", "", + "Kind of log to work on (default: the only one the server offers)") + _ = cmd.RegisterFlagCompletionFunc("kind", baremetal.CompleteLogKind) +} + // addBackupAclProtocolFlags registers the three protocols an access rule can // open. They are registered together because the API takes all three and // accepts all three false — a rule that allows an IP block to reach nothing. diff --git a/internal/cmd/baremetal_logs_test.go b/internal/cmd/baremetal_logs_test.go new file mode 100644 index 000000000..468bb06bc --- /dev/null +++ b/internal/cmd/baremetal_logs_test.go @@ -0,0 +1,267 @@ +// SPDX-FileCopyrightText: 2026 OVH SAS +// +// SPDX-License-Identifier: Apache-2.0 + +package cmd_test + +import ( + "encoding/json" + "net/http" + + "github.com/jarcoal/httpmock" + "github.com/maxatome/go-testdeep/td" + "github.com/ovh/ovhcloud-cli/internal/cmd" +) + +const ( + logsServer = "https://eu.api.ovh.com/v2/dedicated/server/ns1.example" + logsKinds = logsServer + "/log/kind" + logsSubscriptions = logsServer + "/log/subscription" + logsURL = logsServer + "/log/url" +) + +// registerOneLogKind gives the server the single kind the API offers today. +func registerOneLogKind() { + httpmock.RegisterResponder(http.MethodGet, logsKinds, + httpmock.NewStringResponder(200, `["install"]`)) + httpmock.RegisterResponder(http.MethodGet, logsKinds+"/install", + httpmock.NewStringResponder(200, `{"kindId":"75fa0fec-812c-46af-a41b-76d1e3dc2843","name":"install","displayName":"Operating system installation logs","additionalReturnedFields":["level","os","serviceName","status"]}`)) +} + +// registerStreams answers the Log Data Platform sweep with two services, one of +// which carries a title the other one carries too — the case this account +// really has three times over. +func registerStreams() { + httpmock.RegisterResponder(http.MethodGet, "https://eu.api.ovh.com/v1/dbaas/logs", + httpmock.NewStringResponder(200, `["ldp-aa-1","ldp-bb-2"]`)) + httpmock.RegisterResponder(http.MethodGet, "https://eu.api.ovh.com/v1/dbaas/logs/ldp-aa-1/output/graylog/stream", + httpmock.NewStringResponder(200, `["11111111-1111-1111-1111-111111111111","33333333-3333-3333-3333-333333333333"]`)) + httpmock.RegisterResponder(http.MethodGet, "https://eu.api.ovh.com/v1/dbaas/logs/ldp-bb-2/output/graylog/stream", + httpmock.NewStringResponder(200, `["22222222-2222-2222-2222-222222222222"]`)) + httpmock.RegisterResponder(http.MethodGet, "https://eu.api.ovh.com/v1/dbaas/logs/ldp-aa-1/output/graylog/stream/11111111-1111-1111-1111-111111111111", + httpmock.NewStringResponder(200, `{"streamId":"11111111-1111-1111-1111-111111111111","title":"shared"}`)) + httpmock.RegisterResponder(http.MethodGet, "https://eu.api.ovh.com/v1/dbaas/logs/ldp-aa-1/output/graylog/stream/33333333-3333-3333-3333-333333333333", + httpmock.NewStringResponder(200, `{"streamId":"33333333-3333-3333-3333-333333333333","title":"TO REMOVE 1"}`)) + httpmock.RegisterResponder(http.MethodGet, "https://eu.api.ovh.com/v1/dbaas/logs/ldp-bb-2/output/graylog/stream/22222222-2222-2222-2222-222222222222", + httpmock.NewStringResponder(200, `{"streamId":"22222222-2222-2222-2222-222222222222","title":"shared"}`)) +} + +func (ms *MockSuite) TestBaremetalLogKindsSaysWhatAKindHolds(assert, require *td.T) { + registerOneLogKind() + + out, err := cmd.Execute("baremetal", "logs", "kinds", "ns1.example") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains("Operating system installation logs"), + "the name alone would send everybody to the API documentation") + assert.Cmp(out, td.Contains("status"), "and the extra fields say what a search will return") +} + +// A server with no kind is an answer, not an empty table with headers. +func (ms *MockSuite) TestBaremetalLogKindsSaysWhenThereAreNone(assert, require *td.T) { + httpmock.RegisterResponder(http.MethodGet, logsKinds, httpmock.NewStringResponder(200, `[]`)) + + out, err := cmd.Execute("baremetal", "logs", "kinds", "ns1.example") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains("no log kind")) +} + +// The kind filter belongs to the collection call. Carried into the expansion it +// would build ".../log/subscription?kind=install/", a route that does not +// exist, for a filter that would have looked like it worked. +func (ms *MockSuite) TestBaremetalLogSubscriptionsFilterStaysOnTheCollection(assert, require *td.T) { + httpmock.RegisterResponder(http.MethodGet, logsSubscriptions, + httpmock.NewStringResponder(200, `["sub-1"]`)) + httpmock.RegisterResponder(http.MethodGet, logsSubscriptions+"/sub-1", + httpmock.NewStringResponder(200, `{"subscriptionId":"sub-1","kind":"install","streamId":"11111111-1111-1111-1111-111111111111","serviceName":"ldp-aa-1"}`)) + + out, err := cmd.Execute("baremetal", "logs", "subscription", "list", "ns1.example", "--kind", "install") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains("sub-1")) + + for url := range httpmock.GetCallCountInfo() { + assert.Cmp(url, td.Not(td.Contains("kind=install/")), + "the query must not be carried into the per-object URL") + } +} + +// One kind means there is nothing to choose. Making somebody type the only +// possible value is friction with no purpose. +func (ms *MockSuite) TestBaremetalLogSubscribeDoesNotAskForTheOnlyKind(assert, require *td.T) { + registerOneLogKind() + var sent map[string]any + httpmock.RegisterResponder(http.MethodPost, logsSubscriptions, + func(req *http.Request) (*http.Response, error) { + if err := json.NewDecoder(req.Body).Decode(&sent); err != nil { + return nil, err + } + return httpmock.NewStringResponse(200, `{"operationId":"op-1","serviceName":"ldp-aa-1"}`), nil + }) + + _, err := cmd.Execute("baremetal", "logs", "subscribe", "ns1.example", + "--stream", "11111111-1111-1111-1111-111111111111", "--yes") + + require.CmpNoError(err) + assert.Cmp(sent["kind"], "install", "the server was asked, and it had one answer") + assert.Cmp(sent["streamId"], "11111111-1111-1111-1111-111111111111") +} + +// Two kinds means the command must not pick one. The day a second kind appears, +// this is the difference between a refusal and silently subscribing the wrong +// logs. +func (ms *MockSuite) TestBaremetalLogSubscribeRefusesToChooseBetweenKinds(assert, require *td.T) { + httpmock.RegisterResponder(http.MethodGet, logsKinds, + httpmock.NewStringResponder(200, `["install","syslog"]`)) + + _, err := cmd.Execute("baremetal", "logs", "subscribe", "ns1.example", + "--stream", "11111111-1111-1111-1111-111111111111", "--yes") + + require.CmpError(err) + assert.Cmp(err.Error(), td.Contains("--kind")) + assert.Cmp(err.Error(), td.Contains("install, syslog"), "and it names them, so the answer is in the refusal") + assert.Cmp(httpmock.GetCallCountInfo()["POST "+logsSubscriptions], 0, "nothing was sent") +} + +// Three titles on this account are carried by two streams each. Picking one +// would send a machine's logs somewhere nobody asked for. +func (ms *MockSuite) TestBaremetalLogSubscribeRefusesAnAmbiguousTitle(assert, require *td.T) { + registerOneLogKind() + registerStreams() + + _, err := cmd.Execute("baremetal", "logs", "subscribe", "ns1.example", "--stream", "shared", "--yes") + + require.CmpError(err) + assert.Cmp(err.Error(), td.Contains("11111111-1111-1111-1111-111111111111")) + assert.Cmp(err.Error(), td.Contains("22222222-2222-2222-2222-222222222222")) + assert.Cmp(err.Error(), td.Contains("ldp-bb-2"), "the service is what tells the two apart") + assert.Cmp(httpmock.GetCallCountInfo()["POST "+logsSubscriptions], 0) +} + +// A title resolves to the identifier the API is given, and the operator never +// has to know it. +func (ms *MockSuite) TestBaremetalLogSubscribeResolvesATitle(assert, require *td.T) { + registerOneLogKind() + registerStreams() + var sent map[string]any + httpmock.RegisterResponder(http.MethodPost, logsSubscriptions, + func(req *http.Request) (*http.Response, error) { + if err := json.NewDecoder(req.Body).Decode(&sent); err != nil { + return nil, err + } + return httpmock.NewStringResponse(200, `{"operationId":"op-1","serviceName":"ldp-aa-1"}`), nil + }) + + _, err := cmd.Execute("baremetal", "logs", "subscribe", "ns1.example", "--stream", "TO REMOVE 1", "--yes") + + require.CmpNoError(err) + assert.Cmp(sent["streamId"], "33333333-3333-3333-3333-333333333333") +} + +// The operation says the platform finished. The subscription says the logs +// actually go somewhere. Every wait in this CLI reads the second. +func (ms *MockSuite) TestBaremetalLogSubscribeWaitReadsTheSubscriptionBack(assert, require *td.T) { + registerOneLogKind() + httpmock.RegisterResponder(http.MethodPost, logsSubscriptions, + httpmock.NewStringResponder(200, `{"operationId":"op-1","serviceName":"ldp-aa-1"}`)) + httpmock.RegisterResponder(http.MethodGet, "https://eu.api.ovh.com/v1/dbaas/logs/ldp-aa-1/operation/op-1", + httpmock.NewStringResponder(200, `{"operationId":"op-1","state":"SUCCESS","subscriptionId":"sub-9"}`)) + httpmock.RegisterResponder(http.MethodGet, logsSubscriptions+"/sub-9", + httpmock.NewStringResponder(200, `{"subscriptionId":"sub-9","kind":"install","streamId":"11111111-1111-1111-1111-111111111111","serviceName":"ldp-aa-1"}`)) + + out, err := cmd.Execute("baremetal", "logs", "subscribe", "ns1.example", + "--stream", "11111111-1111-1111-1111-111111111111", "--wait", "--yes") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains("sub-9")) + assert.Cmp(httpmock.GetCallCountInfo()["GET "+logsSubscriptions+"/sub-9"], 1, + "the state is read, not assumed from the operation") +} + +// FAILURE and REVOKED both end the operation without doing the work. +func (ms *MockSuite) TestBaremetalLogSubscribeWaitReportsAFailedOperation(assert, require *td.T) { + registerOneLogKind() + httpmock.RegisterResponder(http.MethodPost, logsSubscriptions, + httpmock.NewStringResponder(200, `{"operationId":"op-1","serviceName":"ldp-aa-1"}`)) + httpmock.RegisterResponder(http.MethodGet, "https://eu.api.ovh.com/v1/dbaas/logs/ldp-aa-1/operation/op-1", + httpmock.NewStringResponder(200, `{"operationId":"op-1","state":"FAILURE"}`)) + + _, err := cmd.Execute("baremetal", "logs", "subscribe", "ns1.example", + "--stream", "11111111-1111-1111-1111-111111111111", "--wait", "--yes") + + require.CmpError(err) + assert.Cmp(err.Error(), td.Contains("FAILURE")) + assert.Cmp(err.Error(), td.Not(td.Contains("now go to")), "a failed operation is not a subscription") +} + +// The removal is the same rule the other way round: the operation finishing is +// not the subscription being gone. +func (ms *MockSuite) TestBaremetalLogUnsubscribeWaitChecksItIsActuallyGone(assert, require *td.T) { + httpmock.RegisterResponder(http.MethodGet, logsSubscriptions+"/sub-9", + httpmock.NewStringResponder(200, `{"subscriptionId":"sub-9","kind":"install","streamId":"11111111-1111-1111-1111-111111111111","serviceName":"ldp-aa-1"}`)) + httpmock.RegisterResponder(http.MethodDelete, logsSubscriptions+"/sub-9", + httpmock.NewStringResponder(200, `{"operationId":"op-2","serviceName":"ldp-aa-1"}`)) + httpmock.RegisterResponder(http.MethodGet, "https://eu.api.ovh.com/v1/dbaas/logs/ldp-aa-1/operation/op-2", + httpmock.NewStringResponder(200, `{"operationId":"op-2","state":"SUCCESS"}`)) + + _, err := cmd.Execute("baremetal", "logs", "unsubscribe", "ns1.example", "sub-9", "--wait", "--yes") + + require.CmpError(err) + assert.Cmp(err.Error(), td.Contains("still there"), + "the subscription still answers, so the command must not report it gone") +} + +// An identifier that is not there is not a failure to report as one: it is the +// list somebody needs. +func (ms *MockSuite) TestBaremetalLogUnsubscribeSaysWhatToListWhenTheIdIsWrong(assert, require *td.T) { + httpmock.RegisterResponder(http.MethodGet, logsSubscriptions+"/nope", + httpmock.NewStringResponder(404, `{"message":"not found"}`)) + + _, err := cmd.Execute("baremetal", "logs", "unsubscribe", "ns1.example", "nope", "--yes") + + require.CmpError(err) + assert.Cmp(err.Error(), td.Contains("logs subscription list")) + assert.Cmp(httpmock.GetCallCountInfo()["DELETE "+logsSubscriptions+"/nope"], 0) +} + +// The prompt names what stops, read from the subscription, rather than quoting +// back an identifier the operator just pasted. +func (ms *MockSuite) TestBaremetalLogUnsubscribeNamesWhatItStops(assert, require *td.T) { + httpmock.RegisterResponder(http.MethodGet, logsSubscriptions+"/sub-9", + httpmock.NewStringResponder(200, `{"subscriptionId":"sub-9","kind":"install","streamId":"11111111-1111-1111-1111-111111111111","serviceName":"ldp-aa-1"}`)) + + out, err := cmd.Execute("baremetal", "logs", "unsubscribe", "ns1.example", "sub-9", "--dry-run") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains("DELETE")) + assert.Cmp(httpmock.GetCallCountInfo()["DELETE "+logsSubscriptions+"/sub-9"], 0, "a dry run sends nothing") +} + +// The link is the answer, so it is printed; what it costs is said beside it. +func (ms *MockSuite) TestBaremetalLogUrlPrintsTheLinkAndItsExpiry(assert, require *td.T) { + registerOneLogKind() + httpmock.RegisterResponder(http.MethodPost, logsURL, + httpmock.NewStringResponder(200, `{"url":"https://get.logs.ovh.com/search?plq=abc","expirationDate":"2099-01-01T00:30:00+00:00"}`)) + + out, err := cmd.Execute("baremetal", "logs", "url", "ns1.example") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains("get.logs.ovh.com")) + assert.Cmp(out, td.Contains("2099-01-01T00:30:00+00:00"), "a link that expires in silence is a trap") + assert.Cmp(out, td.Contains("anyone holding it"), "and it carries its own authorisation") +} + +// What an operator checks before agreeing to a subscription is the stream and +// the kind, and neither is in the URL. +func (ms *MockSuite) TestBaremetalLogSubscribeDryRunShowsTheBody(assert, require *td.T) { + registerOneLogKind() + + out, err := cmd.Execute("baremetal", "logs", "subscribe", "ns1.example", + "--stream", "11111111-1111-1111-1111-111111111111", "--dry-run") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains("streamId")) + assert.Cmp(out, td.Contains("install")) + assert.Cmp(httpmock.GetCallCountInfo()["POST "+logsSubscriptions], 0) +} diff --git a/internal/services/baremetal/logs.go b/internal/services/baremetal/logs.go new file mode 100644 index 000000000..3b63de80d --- /dev/null +++ b/internal/services/baremetal/logs.go @@ -0,0 +1,506 @@ +// SPDX-FileCopyrightText: 2026 OVH SAS +// +// SPDX-License-Identifier: Apache-2.0 + +package baremetal + +import ( + "encoding/json" + "fmt" + "net/http" + "net/url" + "sort" + "strings" + "time" + + "github.com/ovh/ovhcloud-cli/internal/display" + "github.com/ovh/ovhcloud-cli/internal/flags" + httpLib "github.com/ovh/ovhcloud-cli/internal/http" + "github.com/ovh/ovhcloud-cli/internal/services/common" + "github.com/spf13/cobra" +) + +// A dedicated server can send its logs to a Log Data Platform stream, and none +// of it was reachable from here. The seven routes live in the v2 catalogue, +// which this CLI could not even fetch a schema for until now, so the surface +// was invisible rather than skipped. +// +// The v2 routes are a thin proxy over LDP rather than a resource of their own: +// they answer with dbaas.logs.* types, and a subscription that is being created +// or removed is followed on the v1 LDP service, not on anything under /v2. So a +// single --wait crosses both catalogues, and the object it needs to do that — +// the LDP service carrying the stream — is handed to it by the API itself. +// +// Every one of these operations is badged "Alpha version" upstream. + +const ( + logKindsPath = "/v2/dedicated/server/%s/log/kind" + logSubscriptionsPath = "/v2/dedicated/server/%s/log/subscription" + logURLPath = "/v2/dedicated/server/%s/log/url" +) + +var ( + // LogKind is the kind of log a command works on. + // + // It is optional. Only one kind exists today — "install", the operating + // system installation logs — and requiring an operator to name the only + // possible value is friction with no purpose. Defaulting to the string + // "install" in Go would be worse: it would keep working, and keep being + // the only kind anyone could reach, on the day a second one appears. So + // the server is asked what it has, and the answer decides. + LogKind string + + // LogStream is the stream to subscribe to, by title or by identifier. + LogStream string + + // LogWait keeps the command running until the operation has finished. + LogWait bool +) + +// Polling settings for the LDP operation behind a subscription change. +// +// Variables rather than constants so a test can exercise the polling itself +// without waiting five seconds a round. Measured against the real API on +// 20 August 2026, a subscription settles in about seven seconds and its removal +// in six, so the five-minute ceiling is wide. +var ( + logPollInterval = 5 * time.Second + logPollAttempts = 60 +) + +// logOperationStates: dbaas.logs.OperationStateEnum, which is a sixth status +// vocabulary in this repository and shares nothing with the five already here. +// FAILURE and REVOKED both end the operation without doing the work; RETRY is +// not an ending, it is the platform trying again. +var ( + logOperationSucceeded = "SUCCESS" + logOperationFailed = map[string]bool{"FAILURE": true, "REVOKED": true} +) + +// ListBaremetalLogKinds shows what a server can send, expanded. +// +// The list route answers with names alone. Each name is then read, because the +// useful part is what a kind actually contains — its display name and the extra +// fields it carries — and a column of one word would send everybody to the API +// documentation. +func ListBaremetalLogKinds(_ *cobra.Command, args []string) { + server := args[0] + + kinds, err := logKindsOf(server) + if err != nil { + display.OutputError(&flags.OutputFormatConfig, "%s", err) + return + } + + if len(kinds) == 0 { + display.OutputInfo(&flags.OutputFormatConfig, + map[string]any{"serviceName": server, "kinds": []any{}}, + "%s has no log kind, so there is nothing to subscribe to yet.", server) + return + } + + path := fmt.Sprintf(logKindsPath, url.PathEscape(server)) + ids := make([]any, len(kinds)) + for index, kind := range kinds { + ids[index] = kind + } + + details, err := httpLib.FetchObjectsParallel[map[string]any](path+"/%s", ids, flags.IgnoreErrors) + if err != nil { + display.OutputError(&flags.OutputFormatConfig, "failed to read the log kinds of %s: %s", server, err) + return + } + + display.RenderTable(details, + []string{"name", "displayName", "additionalReturnedFields", "kindId"}, + &flags.OutputFormatConfig) +} + +// ShowBaremetalLogURL asks for a temporary link to the logs of a server. +// +// It prints a link and says when it dies. The route is named "log/url" and that +// is exactly what it returns: a signed https://get.logs.ovh.com/search address +// into the Log Data Platform search interface, not a socket and not a stream. +// Measured on 20 August 2026, it is valid for thirty minutes — short enough +// that a command which printed it without the expiry would be handing over +// something that stops working while it is still on screen. +func ShowBaremetalLogURL(_ *cobra.Command, args []string) { + server := args[0] + + kind, err := chooseLogKind(server) + if err != nil { + display.OutputError(&flags.OutputFormatConfig, "%s", err) + return + } + + endpoint := fmt.Sprintf(logURLPath, url.PathEscape(server)) + body := map[string]any{"kind": kind} + if reportLogDryRun(http.MethodPost, endpoint, body) { + return + } + + var link struct { + URL string `json:"url"` + ExpirationDate string `json:"expirationDate"` + } + if err := httpLib.Client.Post(endpoint, body, &link); err != nil { + display.OutputError(&flags.OutputFormatConfig, + "failed to get a log link for %s: %s", server, err) + return + } + + display.OutputInfo(&flags.OutputFormatConfig, map[string]any{ + "serviceName": server, + "kind": kind, + "url": link.URL, + "expirationDate": link.ExpirationDate, + }, "%s logs of %s, %s:\n\n%s\n\nThis link carries its own authorisation — anyone holding it can read these logs.", + strings.ToUpper(kind[:1])+kind[1:], server, expiryPhrase(link.ExpirationDate), link.URL) +} + +// ListBaremetalLogSubscriptions lists where the logs of a server go. +func ListBaremetalLogSubscriptions(_ *cobra.Command, args []string) { + server := args[0] + + path := fmt.Sprintf(logSubscriptionsPath, url.PathEscape(server)) + + // The kind filter belongs to the collection call and to nothing else. The + // expansion below builds one URL per subscription as path + "/%s", so a + // query string carried over would produce ".../log/subscription?kind=install/1234" + // — a route that does not exist, for a filter that would have looked like + // it worked. + query := "" + if LogKind != "" { + query = "?kind=" + url.QueryEscape(LogKind) + } + + ids, err := httpLib.FetchArray(path+query, "") + if err != nil { + display.OutputError(&flags.OutputFormatConfig, + "failed to list the log subscriptions of %s: %s", server, err) + return + } + + subscriptions, err := httpLib.FetchObjectsParallel[map[string]any](path+"/%s", ids, flags.IgnoreErrors) + if err != nil { + display.OutputError(&flags.OutputFormatConfig, + "failed to read the log subscriptions of %s: %s", server, err) + return + } + + display.RenderTable(subscriptions, + []string{"subscriptionId", "kind", "streamId", "serviceName", "createdAt"}, + &flags.OutputFormatConfig) +} + +// ShowBaremetalLogSubscription reads one subscription. +func ShowBaremetalLogSubscription(_ *cobra.Command, args []string) { + server, id := args[0], args[1] + common.ManageObjectRequest(fmt.Sprintf(logSubscriptionsPath, url.PathEscape(server)), id, "") +} + +// SubscribeBaremetalLogs starts sending the logs of a server to a stream. +func SubscribeBaremetalLogs(_ *cobra.Command, args []string) { + server := args[0] + + kind, err := chooseLogKind(server) + if err != nil { + display.OutputError(&flags.OutputFormatConfig, "%s", err) + return + } + + stream, err := resolveStream(LogStream) + if err != nil { + display.OutputError(&flags.OutputFormatConfig, "%s", err) + return + } + + // Disruptive rather than destructive: nothing is lost, but the logs of a + // machine start landing somewhere new and the indexing they cause is + // billed on the receiving service. + if !common.ConfirmAction(common.Disruptive, server, fmt.Sprintf( + "This sends the %s logs of %s to stream %s. Indexing them is billed on the receiving Log Data Platform service.", + kind, server, streamLabel(stream))) { + display.OutputError(&flags.OutputFormatConfig, "subscription of %s cancelled", server) + return + } + + endpoint := fmt.Sprintf(logSubscriptionsPath, url.PathEscape(server)) + body := map[string]any{"kind": kind, "streamId": stream.StreamID} + if reportLogDryRun(http.MethodPost, endpoint, body) { + return + } + + var response struct { + OperationID string `json:"operationId"` + ServiceName string `json:"serviceName"` + } + if err := httpLib.Client.Post(endpoint, body, &response); err != nil { + display.OutputError(&flags.OutputFormatConfig, + "failed to subscribe %s to stream %s: %s", server, stream.StreamID, err) + return + } + + if !LogWait { + display.OutputInfo(&flags.OutputFormatConfig, response, + "⚡️ The %s logs of %s are being subscribed to stream %s. Follow it with: ovhcloud baremetal logs subscription list %s", + kind, server, stream.StreamID, server) + return + } + + operation, err := waitForLogOperation(response.ServiceName, response.OperationID) + if err != nil { + display.OutputError(&flags.OutputFormatConfig, "%s", err) + return + } + + // The operation reporting SUCCESS is not the subscription existing. Every + // wait in this CLI reads the state afterwards, and this one is handed the + // identifier to read it with: the operation object carries subscriptionId. + subscriptionID, _ := operation["subscriptionId"].(string) + if subscriptionID == "" { + display.OutputInfo(&flags.OutputFormatConfig, operation, + "✅ The operation finished, but it did not say which subscription it created. List them with: ovhcloud baremetal logs subscription list %s", + server) + return + } + + var subscription map[string]any + path := fmt.Sprintf("%s/%s", endpoint, url.PathEscape(subscriptionID)) + if err := httpLib.Client.Get(path, &subscription); err != nil { + display.OutputError(&flags.OutputFormatConfig, + "the operation finished but subscription %s cannot be read back: %s", subscriptionID, err) + return + } + + display.OutputInfo(&flags.OutputFormatConfig, subscription, + "✅ The %s logs of %s now go to stream %s (subscription %s).", + kind, server, stream.StreamID, subscriptionID) +} + +// UnsubscribeBaremetalLogs stops sending the logs of a server to a stream. +func UnsubscribeBaremetalLogs(_ *cobra.Command, args []string) { + server, id := args[0], args[1] + + endpoint := fmt.Sprintf(logSubscriptionsPath, url.PathEscape(server)) + path := fmt.Sprintf("%s/%s", endpoint, url.PathEscape(id)) + + // Read it first, so the prompt says what is about to stop rather than + // quoting back an identifier the operator just pasted. + var subscription map[string]any + if err := httpLib.Client.Get(path, &subscription); err != nil { + if common.IsNotFound(err) { + display.OutputError(&flags.OutputFormatConfig, + "%s has no log subscription %s.\n List them with: ovhcloud baremetal logs subscription list %s", + server, id, server) + return + } + + display.OutputError(&flags.OutputFormatConfig, "failed to read subscription %s: %s", id, err) + return + } + + kind, _ := subscription["kind"].(string) + streamID, _ := subscription["streamId"].(string) + ldpService, _ := subscription["serviceName"].(string) + + if !common.ConfirmAction(common.Disruptive, server, fmt.Sprintf( + "This stops the %s logs of %s reaching stream %s on %s. Logs produced from now on are not collected.", + kind, server, streamID, ldpService)) { + display.OutputError(&flags.OutputFormatConfig, "removal of subscription %s cancelled", id) + return + } + + if reportLogDryRun(http.MethodDelete, path, nil) { + return + } + + var response struct { + OperationID string `json:"operationId"` + ServiceName string `json:"serviceName"` + } + if err := httpLib.Client.Delete(path, &response); err != nil { + display.OutputError(&flags.OutputFormatConfig, + "failed to remove subscription %s: %s", id, err) + return + } + + if !LogWait { + display.OutputInfo(&flags.OutputFormatConfig, response, + "⚡️ Subscription %s of %s is being removed.", id, server) + return + } + + if _, err := waitForLogOperation(response.ServiceName, response.OperationID); err != nil { + display.OutputError(&flags.OutputFormatConfig, "%s", err) + return + } + + // Same rule the other way round: the operation says it finished, the state + // says whether the subscription is gone. + if err := httpLib.Client.Get(path, &subscription); err == nil { + display.OutputError(&flags.OutputFormatConfig, + "the operation finished but subscription %s is still there; read it with: ovhcloud baremetal logs subscription get %s %s", + id, server, id) + return + } else if !common.IsNotFound(err) { + display.OutputError(&flags.OutputFormatConfig, + "the operation finished but subscription %s cannot be checked: %s", id, err) + return + } + + display.OutputInfo(&flags.OutputFormatConfig, + map[string]any{"serviceName": server, "subscriptionId": id}, + "✅ Subscription %s of %s is gone.", id, server) +} + +// logKindsOf lists the kinds of log a server can send. +func logKindsOf(server string) ([]string, error) { + var kinds []string + + path := fmt.Sprintf(logKindsPath, url.PathEscape(server)) + if err := httpLib.Client.Get(path, &kinds); err != nil { + return nil, fmt.Errorf("failed to list the log kinds of %s: %w", server, err) + } + + sort.Strings(kinds) + + return kinds, nil +} + +// chooseLogKind settles which kind a command works on. +// +// One kind means there is nothing to choose, and saying so is better than +// making somebody type it. Several means the command must not pick for them. +// None means this server sends nothing, which is an answer and not a failure of +// the command. +func chooseLogKind(server string) (string, error) { + if LogKind != "" { + return LogKind, nil + } + + kinds, err := logKindsOf(server) + if err != nil { + return "", err + } + + switch len(kinds) { + case 1: + return kinds[0], nil + + case 0: + return "", fmt.Errorf("%s offers no kind of log, so there is nothing to read or subscribe to", server) + + default: + return "", fmt.Errorf("%s offers %d kinds of log; name one with --kind: %s", + server, len(kinds), strings.Join(kinds, ", ")) + } +} + +// waitForLogOperation follows a subscription change to its end. +// +// The change is made against the server on /v2 and returns an operation +// identifier, and no v2 route can follow it: it is followed on the v1 Log Data +// Platform service. The response says which one, so the caller does not have to +// work it out — the same field, serviceName, that a subscription object carries. +func waitForLogOperation(ldpService, operationID string) (map[string]any, error) { + if ldpService == "" || operationID == "" { + return nil, fmt.Errorf("the API accepted the change but did not say how to follow it (service %q, operation %q)", + ldpService, operationID) + } + + path := fmt.Sprintf("/v1/dbaas/logs/%s/operation/%s", + url.PathEscape(ldpService), url.PathEscape(operationID)) + + var last string + for attempt := 0; attempt < logPollAttempts; attempt++ { + var operation map[string]any + if err := httpLib.Client.Get(path, &operation); err != nil { + return nil, fmt.Errorf("failed to follow operation %s on %s: %w", operationID, ldpService, err) + } + + state, _ := operation["state"].(string) + last = state + + switch { + case state == logOperationSucceeded: + return operation, nil + case logOperationFailed[state]: + return nil, fmt.Errorf("operation %s on %s ended in %s", operationID, ldpService, state) + } + + time.Sleep(logPollInterval) + } + + return nil, fmt.Errorf("stopped waiting after %s; operation %s on %s still reports %q", + time.Duration(logPollAttempts)*logPollInterval, operationID, ldpService, last) +} + +// streamLabel names a stream the way the operator asked for it. +// +// When a title was resolved, the title is what they recognise and the +// identifier is what the API acted on, so the prompt shows both. When they +// pasted an identifier, repeating it twice says nothing. +func streamLabel(stream ldpStream) string { + if stream.Title == "" { + return stream.StreamID + } + + return fmt.Sprintf("%q (%s on %s)", stream.Title, stream.StreamID, stream.ServiceName) +} + +// expiryPhrase says how long a temporary link has left, in the words somebody +// reads rather than as a timestamp they have to subtract from now. +func expiryPhrase(expiration string) string { + if expiration == "" { + return "with no stated expiry" + } + + deadline, err := time.Parse(time.RFC3339, expiration) + if err != nil { + return "valid until " + expiration + } + + remaining := time.Until(deadline).Round(time.Minute) + if remaining <= 0 { + return "already expired (" + expiration + ")" + } + + return fmt.Sprintf("valid for %s, until %s", remaining, expiration) +} + +// reportLogDryRun previews a call with the body it would carry. +// +// common.ReportDryRun shows the method and the path, which is the whole story +// for a DELETE and half of it for these two POSTs: what an operator needs to +// check before agreeing to a subscription is which stream and which kind, and +// neither is in the URL. Printing them as a second message would mean two JSON +// documents on one stdout under -o json, so this builds the one document +// itself — the same reason `baremetal ticket` does. +func reportLogDryRun(method, endpoint string, body map[string]any) bool { + if !flags.DryRun { + return false + } + + message := fmt.Sprintf("🔍 Dry run: nothing was sent. This would have been called:\n %s %s", method, endpoint) + details := map[string]any{ + "calls": []map[string]any{{"method": method, "endpoint": endpoint}}, + } + + if len(body) > 0 { + rendered, err := json.MarshalIndent(body, " ", " ") + if err != nil { + // Nothing here can fail to marshal — the maps are strings — but a + // preview that swallowed an error would be a preview that lied. + display.OutputError(&flags.OutputFormatConfig, "failed to render the request body: %s", err) + return true + } + + message += fmt.Sprintf("\n\nwith:\n %s", rendered) + details["payload"] = body + } + + display.OutputInfo(&flags.OutputFormatConfig, details, "%s", message) + + return true +} diff --git a/internal/services/baremetal/logs_stream.go b/internal/services/baremetal/logs_stream.go new file mode 100644 index 000000000..8c5449714 --- /dev/null +++ b/internal/services/baremetal/logs_stream.go @@ -0,0 +1,300 @@ +// SPDX-FileCopyrightText: 2026 OVH SAS +// +// SPDX-License-Identifier: Apache-2.0 + +package baremetal + +import ( + "fmt" + "net/url" + "sort" + "strings" + "sync" + "time" + + "github.com/ovh/ovhcloud-cli/internal/cache" + httpLib "github.com/ovh/ovhcloud-cli/internal/http" + "github.com/spf13/cobra" + "golang.org/x/sync/errgroup" +) + +// Subscribing a server to its logs means naming a Graylog stream, and a stream +// is named by a UUID. Nobody has one to hand: this account carries 25 Log Data +// Platform services and 59 streams between them, one service holding 21 on its +// own, and the streams are how a person thinks of them — "Prestashop nginx logs +// (filebeat)", "datastream_test". So a title is accepted and resolved here, +// exactly as `vrack attach` resolves a server to its interface and `baremetal +// traffic` resolves it to its network controllers. +// +// The sweep costs 85 requests. Measured on 20 August 2026 against the real +// account: 2.15 seconds with the parallelism below. That is cheap enough that +// none of it is cached — a subscription is a write, and a stale identifier +// resolved from a cache would send a machine's logs to the wrong place. The +// completion helper caches its suggestions, because a stale suggestion is only +// a suggestion. + +// ldpStream is one Graylog stream and the Log Data Platform service holding it. +// +// The service travels with the stream because everything downstream needs it: +// the subscription is created against the server but its operation is followed +// on the LDP service, and a title alone does not say which service that is. +type ldpStream struct { + ServiceName string + StreamID string + Title string +} + +// listLdpStreams reads every stream of every LDP service on the account. +func listLdpStreams() ([]ldpStream, error) { + var services []string + if err := httpLib.Client.Get("/v1/dbaas/logs", &services); err != nil { + return nil, fmt.Errorf("failed to list the Log Data Platform services: %w", err) + } + + if len(services) == 0 { + return nil, nil + } + + // Two rounds, both bounded: the stream identifiers of each service, then + // the object behind each identifier, which is the only place the title is. + type pair struct{ service, stream string } + + var ( + mutex sync.Mutex + pairs []pair + ) + + listing := new(errgroup.Group) + listing.SetLimit(10) + for _, service := range services { + listing.Go(func() error { + var ids []string + path := fmt.Sprintf("/v1/dbaas/logs/%s/output/graylog/stream", url.PathEscape(service)) + if err := httpLib.Client.Get(path, &ids); err != nil { + return fmt.Errorf("failed to list the streams of %s: %w", service, err) + } + + mutex.Lock() + defer mutex.Unlock() + for _, id := range ids { + pairs = append(pairs, pair{service: service, stream: id}) + } + + return nil + }) + } + if err := listing.Wait(); err != nil { + return nil, err + } + + streams := make([]ldpStream, len(pairs)) + reading := new(errgroup.Group) + reading.SetLimit(10) + for index, p := range pairs { + reading.Go(func() error { + var object struct { + StreamID string `json:"streamId"` + Title string `json:"title"` + } + + path := fmt.Sprintf("/v1/dbaas/logs/%s/output/graylog/stream/%s", + url.PathEscape(p.service), url.PathEscape(p.stream)) + if err := httpLib.Client.Get(path, &object); err != nil { + return fmt.Errorf("failed to read the stream %s of %s: %w", p.stream, p.service, err) + } + + streams[index] = ldpStream{ + ServiceName: p.service, + StreamID: object.StreamID, + Title: object.Title, + } + + return nil + }) + } + if err := reading.Wait(); err != nil { + return nil, err + } + + sort.Slice(streams, func(i, j int) bool { + if streams[i].Title != streams[j].Title { + return streams[i].Title < streams[j].Title + } + return streams[i].ServiceName < streams[j].ServiceName + }) + + return streams, nil +} + +// resolveStream turns what the operator typed into a stream and its service. +// +// A UUID is taken as given and costs nothing: that is what -o json hands back, +// and re-reading 85 objects to confirm an identifier the caller already has +// would be work done to learn nothing. Anything else is a title, matched +// exactly — this account holds streams called "Test" and "test", so a +// case-insensitive match would silently pick one of two different streams. +func resolveStream(wanted string) (ldpStream, error) { + if looksLikeUUID(wanted) { + return ldpStream{StreamID: wanted}, nil + } + + streams, err := listLdpStreams() + if err != nil { + return ldpStream{}, err + } + + if len(streams) == 0 { + return ldpStream{}, fmt.Errorf( + "this account has no Log Data Platform service, so there is no stream to subscribe to.\n"+ + " A subscription needs one; see `ovhcloud ldp list` once you have ordered it (looking for %q)", wanted) + } + + var matches []ldpStream + for _, stream := range streams { + if stream.Title == wanted { + matches = append(matches, stream) + } + } + + switch len(matches) { + case 1: + return matches[0], nil + + case 0: + return ldpStream{}, fmt.Errorf( + "no stream is called %q. %s\n List them with: ovhcloud ldp list", + wanted, nearbyTitles(streams, wanted)) + + default: + var lines []string + for _, match := range matches { + lines = append(lines, fmt.Sprintf(" %s (service %s)", match.StreamID, match.ServiceName)) + } + + return ldpStream{}, fmt.Errorf( + "%d streams are called %q, on different Log Data Platform services.\n"+ + " Name the one you mean by its identifier:\n%s", + len(matches), wanted, strings.Join(lines, "\n")) + } +} + +// nearbyTitles says what does exist, so a typo does not end in a bare refusal. +// +// It only offers titles that contain what was typed, or are contained by it: a +// list of 56 unrelated names would be noise, and an edit distance would suggest +// a stream that has nothing to do with the request. +func nearbyTitles(streams []ldpStream, wanted string) string { + folded := strings.ToLower(wanted) + + var near []string + for _, stream := range streams { + title := strings.ToLower(stream.Title) + if title == "" { + continue + } + if strings.Contains(title, folded) || strings.Contains(folded, title) { + near = append(near, fmt.Sprintf("%q (service %s)", stream.Title, stream.ServiceName)) + } + } + + if len(near) == 0 { + return fmt.Sprintf("This account has %d of them.", len(streams)) + } + + // Five is enough to recognise a typo. The rest are counted rather than + // dropped in silence: a list that stops without saying so reads as the + // whole answer. + const shown = 5 + if len(near) > shown { + return fmt.Sprintf("Close to: %s, and %d more.", strings.Join(near[:shown], ", "), len(near)-shown) + } + + return "Close to: " + strings.Join(near, ", ") + "." +} + +// looksLikeUUID recognises the shape of a stream identifier. +// +// It is deliberately a shape test and not a parse: the point is to tell "the +// caller gave me an identifier" from "the caller gave me a title", and a title +// that happens to be 36 characters of hex and dashes is not a case worth a +// dependency. +func looksLikeUUID(candidate string) bool { + if len(candidate) != 36 { + return false + } + + for index, char := range candidate { + switch index { + case 8, 13, 18, 23: + if char != '-' { + return false + } + default: + isDigit := char >= '0' && char <= '9' + isLower := char >= 'a' && char <= 'f' + isUpper := char >= 'A' && char <= 'F' + if !isDigit && !isLower && !isUpper { + return false + } + } + } + + return true +} + +// CompleteLogStream suggests the titles of the streams on the account. +// +// This is the one place the sweep is cached. A suggestion that is ten minutes +// stale costs a that offers a stream somebody deleted; the resolution +// above is not cached, because there the same staleness would send a machine's +// logs to the wrong place. +func CompleteLogStream(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { + const ( + namespace = "completion" + key = "baremetal-log-stream-titles" + ttl = 10 * time.Minute + ) + + if data, found := cache.Read(namespace, key, ttl); found { + trimmed := strings.Trim(string(data), "\n") + if trimmed == "" { + return nil, cobra.ShellCompDirectiveNoFileComp + } + return strings.Split(trimmed, "\n"), cobra.ShellCompDirectiveNoFileComp + } + + streams, err := listLdpStreams() + if err != nil { + return nil, cobra.ShellCompDirectiveError + } + + // A title with a space in it cannot be offered as a bare word, and a title + // with a newline in it would corrupt the cache file. Both are dropped + // rather than mangled: completion is a convenience, and a suggestion that + // does not work when accepted is worse than no suggestion. + var titles []string + for _, stream := range streams { + if stream.Title == "" || strings.ContainsAny(stream.Title, " \t\n") { + continue + } + titles = append(titles, stream.Title) + } + + cache.Write(namespace, key, []byte(strings.Join(titles, "\n")), ttl) + + return titles, cobra.ShellCompDirectiveNoFileComp +} + +// CompleteLogKind suggests the kinds of log the server being named can send. +func CompleteLogKind(_ *cobra.Command, args []string, _ string) ([]string, cobra.ShellCompDirective) { + if len(args) == 0 { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + kinds, err := logKindsOf(args[0]) + if err != nil { + return nil, cobra.ShellCompDirectiveError + } + + return kinds, cobra.ShellCompDirectiveNoFileComp +} diff --git a/internal/services/baremetal/logs_test.go b/internal/services/baremetal/logs_test.go new file mode 100644 index 000000000..e8f4f3da9 --- /dev/null +++ b/internal/services/baremetal/logs_test.go @@ -0,0 +1,131 @@ +// SPDX-FileCopyrightText: 2026 OVH SAS +// +// SPDX-License-Identifier: Apache-2.0 + +package baremetal + +import ( + "strings" + "testing" + "time" +) + +// A stream is named by a title or by a UUID, and the two are told apart by +// shape alone. Getting this wrong in either direction is silent: a UUID +// mistaken for a title triggers a sweep that finds nothing, and a title +// mistaken for a UUID is sent to the API as a stream identifier. +func TestAStreamIdentifierIsRecognisedByItsShape(t *testing.T) { + if !looksLikeUUID("4459af12-6451-45de-808b-2b959c11a17e") { + t.Fatal("a real stream identifier must be taken as one") + } + if !looksLikeUUID("4459AF12-6451-45DE-808B-2B959C11A17E") { + t.Fatal("the API answers in lowercase but accepts either") + } + + for _, notAnIdentifier := range []string{ + "TO REMOVE 1", + "datastream_test", + "", + "4459af12-6451-45de-808b-2b959c11a17", // one short + "4459af12-6451-45de-808b-2b959c11a17ee", // one long + "4459af12x6451-45de-808b-2b959c11a17e", // separator moved + "zzzzzzzz-6451-45de-808b-2b959c11a17e", // not hexadecimal + "Prestashop nginx logs (filebeat) padded to 36", // right length, wrong everything + } { + if looksLikeUUID(notAnIdentifier) { + t.Fatalf("%q must be treated as a title, not an identifier", notAnIdentifier) + } + } +} + +// A UUID is taken as given: re-reading 85 objects to confirm an identifier the +// caller already holds is work done to learn nothing, and it is what `-o json` +// hands back. +func TestAnIdentifierIsUsedWithoutASweep(t *testing.T) { + stream, err := resolveStream("4459af12-6451-45de-808b-2b959c11a17e") + if err != nil { + t.Fatalf("an identifier must resolve to itself: %s", err) + } + if stream.StreamID != "4459af12-6451-45de-808b-2b959c11a17e" { + t.Fatalf("got %q", stream.StreamID) + } + if stream.Title != "" || stream.ServiceName != "" { + t.Fatal("nothing was read, so nothing but the identifier may be claimed") + } +} + +// The account this was built against holds 59 streams. A refusal that listed +// them all would answer worse than one that names the near misses and counts +// the rest. +func TestNearbyTitlesNamesAFewAndCountsTheRest(t *testing.T) { + var streams []ldpStream + for _, title := range []string{ + "Stream1", "Stream 2", "Substream1a", "My first data stream", + "Data-Stream-Pierrick-PCC", "datastream_test", "Test datastream", + } { + streams = append(streams, ldpStream{Title: title, ServiceName: "ldp-xx-1"}) + } + + near := nearbyTitles(streams, "Stream") + if !strings.Contains(near, "and 2 more") { + t.Fatalf("a truncated list has to say it was truncated, got %q", near) + } + if strings.Count(near, "ldp-xx-1") != 5 { + t.Fatalf("five near misses were expected, got %q", near) + } + + // Nothing close: say how many exist rather than pretend to suggest. + if far := nearbyTitles(streams, "zzzz"); !strings.Contains(far, "7 of them") { + t.Fatalf("got %q", far) + } +} + +// A stream is named to the operator the way they named it. When they typed a +// title, the identifier the API acted on is shown beside it; when they pasted +// an identifier, repeating it says nothing. +func TestAStreamIsNamedTheWayItWasAskedFor(t *testing.T) { + titled := streamLabel(ldpStream{Title: "TO REMOVE 1", StreamID: "69813166-7b66-4a09-8fcd-c47781d23966", ServiceName: "ldp-nx-19421"}) + if !strings.Contains(titled, "TO REMOVE 1") || !strings.Contains(titled, "ldp-nx-19421") { + t.Fatalf("got %q", titled) + } + + bare := streamLabel(ldpStream{StreamID: "69813166-7b66-4a09-8fcd-c47781d23966"}) + if bare != "69813166-7b66-4a09-8fcd-c47781d23966" { + t.Fatalf("got %q", bare) + } +} + +// The link is valid for thirty minutes. A command that printed it without the +// expiry would hand over something that stops working while it is on screen. +func TestExpiryIsSaidInMinutesLeft(t *testing.T) { + soon := time.Now().Add(29*time.Minute + 40*time.Second).UTC().Format(time.RFC3339) + if phrase := expiryPhrase(soon); !strings.Contains(phrase, "30m0s") { + t.Fatalf("got %q", phrase) + } + + past := time.Now().Add(-time.Hour).UTC().Format(time.RFC3339) + if phrase := expiryPhrase(past); !strings.Contains(phrase, "already expired") { + t.Fatalf("an expired link must say so, got %q", phrase) + } + + // An unreadable date is still shown: the API said something, and dropping + // it would leave a link with no expiry at all. + if phrase := expiryPhrase("not a date"); !strings.Contains(phrase, "not a date") { + t.Fatalf("got %q", phrase) + } + if phrase := expiryPhrase(""); !strings.Contains(phrase, "no stated expiry") { + t.Fatalf("got %q", phrase) + } +} + +// The change is made on /v2 and followed on /v1, and the only thing joining the +// two is what the API answered. If it answered neither, the wait has nothing to +// poll and has to say so instead of looping on an empty path. +func TestAWaitWithNothingToFollowSaysSo(t *testing.T) { + if _, err := waitForLogOperation("", "op-1"); err == nil { + t.Fatal("a wait without a Log Data Platform service must refuse") + } + if _, err := waitForLogOperation("ldp-nx-19421", ""); err == nil { + t.Fatal("a wait without an operation must refuse") + } +} From 8a50b0c5b6a54c525d0506b5b8bf19e14b18e029 Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Thu, 20 Aug 2026 16:14:56 +0200 Subject: [PATCH 04/11] chore: no account data as test fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A hostname, an address, an order id and a colleague's name read off a live account had been used as fixtures and as examples in comments. This repository is public: what goes in stays in. The values are replaced with synthetic ones of the same shape — RFC 5737 documentation addresses, hostnames built on them, identifiers of the same length — so the tests keep exercising the same parsing. This is the fourth time in this series that live account data reached a commit, after a live IP migration token, a test server's address, and a real invoice id. The first three were fixed one at a time as they were noticed; this is the sweep that should have followed the first one. Signed-off-by: Denis Hamon Co-Authored-By: Claude Opus 5 (1M context) --- internal/services/baremetal/logs_test.go | 24 ++++++++++++------------ internal/services/ip/move_test.go | 2 +- internal/services/vrack/attach_test.go | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/internal/services/baremetal/logs_test.go b/internal/services/baremetal/logs_test.go index e8f4f3da9..1991b25a3 100644 --- a/internal/services/baremetal/logs_test.go +++ b/internal/services/baremetal/logs_test.go @@ -15,10 +15,10 @@ import ( // mistaken for a title triggers a sweep that finds nothing, and a title // mistaken for a UUID is sent to the API as a stream identifier. func TestAStreamIdentifierIsRecognisedByItsShape(t *testing.T) { - if !looksLikeUUID("4459af12-6451-45de-808b-2b959c11a17e") { + if !looksLikeUUID("00000000-6451-45de-808b-2b959c11a17e") { t.Fatal("a real stream identifier must be taken as one") } - if !looksLikeUUID("4459AF12-6451-45DE-808B-2B959C11A17E") { + if !looksLikeUUID("00000000-6451-45DE-808B-2B959C11A17E") { t.Fatal("the API answers in lowercase but accepts either") } @@ -26,9 +26,9 @@ func TestAStreamIdentifierIsRecognisedByItsShape(t *testing.T) { "TO REMOVE 1", "datastream_test", "", - "4459af12-6451-45de-808b-2b959c11a17", // one short - "4459af12-6451-45de-808b-2b959c11a17ee", // one long - "4459af12x6451-45de-808b-2b959c11a17e", // separator moved + "00000000-6451-45de-808b-2b959c11a17", // one short + "00000000-6451-45de-808b-2b959c11a17ee", // one long + "00000000x6451-45de-808b-2b959c11a17e", // separator moved "zzzzzzzz-6451-45de-808b-2b959c11a17e", // not hexadecimal "Prestashop nginx logs (filebeat) padded to 36", // right length, wrong everything } { @@ -42,11 +42,11 @@ func TestAStreamIdentifierIsRecognisedByItsShape(t *testing.T) { // caller already holds is work done to learn nothing, and it is what `-o json` // hands back. func TestAnIdentifierIsUsedWithoutASweep(t *testing.T) { - stream, err := resolveStream("4459af12-6451-45de-808b-2b959c11a17e") + stream, err := resolveStream("00000000-6451-45de-808b-2b959c11a17e") if err != nil { t.Fatalf("an identifier must resolve to itself: %s", err) } - if stream.StreamID != "4459af12-6451-45de-808b-2b959c11a17e" { + if stream.StreamID != "00000000-6451-45de-808b-2b959c11a17e" { t.Fatalf("got %q", stream.StreamID) } if stream.Title != "" || stream.ServiceName != "" { @@ -84,13 +84,13 @@ func TestNearbyTitlesNamesAFewAndCountsTheRest(t *testing.T) { // title, the identifier the API acted on is shown beside it; when they pasted // an identifier, repeating it says nothing. func TestAStreamIsNamedTheWayItWasAskedFor(t *testing.T) { - titled := streamLabel(ldpStream{Title: "TO REMOVE 1", StreamID: "69813166-7b66-4a09-8fcd-c47781d23966", ServiceName: "ldp-nx-19421"}) - if !strings.Contains(titled, "TO REMOVE 1") || !strings.Contains(titled, "ldp-nx-19421") { + titled := streamLabel(ldpStream{Title: "TO REMOVE 1", StreamID: "00000000-0000-4000-8000-000000000001", ServiceName: "ldp-xx-00000"}) + if !strings.Contains(titled, "TO REMOVE 1") || !strings.Contains(titled, "ldp-xx-00000") { t.Fatalf("got %q", titled) } - bare := streamLabel(ldpStream{StreamID: "69813166-7b66-4a09-8fcd-c47781d23966"}) - if bare != "69813166-7b66-4a09-8fcd-c47781d23966" { + bare := streamLabel(ldpStream{StreamID: "00000000-0000-4000-8000-000000000001"}) + if bare != "00000000-0000-4000-8000-000000000001" { t.Fatalf("got %q", bare) } } @@ -125,7 +125,7 @@ func TestAWaitWithNothingToFollowSaysSo(t *testing.T) { if _, err := waitForLogOperation("", "op-1"); err == nil { t.Fatal("a wait without a Log Data Platform service must refuse") } - if _, err := waitForLogOperation("ldp-nx-19421", ""); err == nil { + if _, err := waitForLogOperation("ldp-xx-00000", ""); err == nil { t.Fatal("a wait without an operation must refuse") } } 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 7b96f46a6f69f0080db2d0a7072c64980b2b9733 Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Thu, 20 Aug 2026 22:49:45 +0200 Subject: [PATCH 05/11] fix(baremetal): --filter was accepted, documented, and ignored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `baremetal logs subscription list` registered the flag and rendered its rows directly. withFilterFlag only binds --filter to flags.GenericFilters; display.RenderTable does not read it, and ManageListRequest — which normally passes it to filters.FilterLines — is not on this path because the rows are assembled here. So the flag was accepted by cobra, written into doc/ by docgen, and inert. An operator reading a list they believe was narrowed is worse served than one who was never offered the flag. These go through common.RenderFilteredTable, added for the same defect earlier in the stack. The test asserts the row the filter EXCLUDES is absent, not merely that the kept one is present: the second passes just as well with the filtering removed. Found by a review pass over this branch, then confirmed by an audit of every withFilterFlag call site in the tree — twenty-one commands this series added carried the same defect. Signed-off-by: Denis Hamon Co-Authored-By: Claude Opus 5 (1M context) --- internal/cmd/baremetal_logs_test.go | 19 +++++++++++++++++++ internal/services/baremetal/logs.go | 5 ++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/internal/cmd/baremetal_logs_test.go b/internal/cmd/baremetal_logs_test.go index 468bb06bc..b0309c90e 100644 --- a/internal/cmd/baremetal_logs_test.go +++ b/internal/cmd/baremetal_logs_test.go @@ -265,3 +265,22 @@ func (ms *MockSuite) TestBaremetalLogSubscribeDryRunShowsTheBody(assert, require assert.Cmp(out, td.Contains("install")) assert.Cmp(httpmock.GetCallCountInfo()["POST "+logsSubscriptions], 0) } + +// --filter is registered on this command, so it has to reach the rows. The +// assertion that carries the test is the absence of the excluded subscription: +// checking only the kept one would pass just as well with no filtering at all. +func (ms *MockSuite) TestBaremetalLogSubscriptionsAreFiltered(assert, require *td.T) { + httpmock.RegisterResponder(http.MethodGet, logsSubscriptions, + httpmock.NewStringResponder(200, `["sub-1","sub-2"]`)) + httpmock.RegisterResponder(http.MethodGet, logsSubscriptions+"/sub-1", + httpmock.NewStringResponder(200, `{"subscriptionId":"sub-1","kind":"install","streamId":"11111111-1111-1111-1111-111111111111","serviceName":"ldp-aa-1"}`)) + httpmock.RegisterResponder(http.MethodGet, logsSubscriptions+"/sub-2", + httpmock.NewStringResponder(200, `{"subscriptionId":"sub-2","kind":"install","streamId":"22222222-2222-2222-2222-222222222222","serviceName":"ldp-bb-2"}`)) + + out, err := cmd.Execute("baremetal", "logs", "subscription", "list", "ns1.example", + "--filter", `serviceName=="ldp-bb-2"`) + + require.CmpNoError(err) + assert.Cmp(out, td.Contains("sub-2")) + assert.Cmp(out, td.Not(td.Contains("sub-1")), "the subscription the filter excludes must not be printed") +} diff --git a/internal/services/baremetal/logs.go b/internal/services/baremetal/logs.go index 3b63de80d..ad51eec33 100644 --- a/internal/services/baremetal/logs.go +++ b/internal/services/baremetal/logs.go @@ -188,9 +188,8 @@ func ListBaremetalLogSubscriptions(_ *cobra.Command, args []string) { return } - display.RenderTable(subscriptions, - []string{"subscriptionId", "kind", "streamId", "serviceName", "createdAt"}, - &flags.OutputFormatConfig) + common.RenderFilteredTable(subscriptions, + []string{"subscriptionId", "kind", "streamId", "serviceName", "createdAt"}) } // ShowBaremetalLogSubscription reads one subscription. From f9cd6cba6b6e13757fa71c3ac70ec62a4f87d47c Mon Sep 17 00:00:00 2001 From: Denis Hamon Date: Fri, 21 Aug 2026 12:43:37 +0200 Subject: [PATCH 06/11] fix(baremetal): the stream refusal named a command that lists something else MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An unknown stream title sent the operator to `ovhcloud ldp list`, which lists Log Data Platform services — `ldp-aa-1`, `ldp-bb-2` — and not Graylog streams. No command of this tree lists streams at all; the only surface that exposes them is the completion on this very flag. So the remedy pointed at the wrong object, on an account where 59 streams live across 25 services and three titles are carried by two streams each. It points at completion now. nearbyTitles already says how many there are and which ones are close, so nothing is lost. The sibling refusal ten lines up does name `ldp list`, and correctly: there it is talking about services. One test, red under a sabotage that puts the old sentence back. Signed-off-by: Denis Hamon Co-Authored-By: Claude Opus 5 (1M context) --- internal/cmd/baremetal_logs_test.go | 17 +++++++++++++++++ internal/services/baremetal/logs_stream.go | 6 +++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/internal/cmd/baremetal_logs_test.go b/internal/cmd/baremetal_logs_test.go index b0309c90e..93192252c 100644 --- a/internal/cmd/baremetal_logs_test.go +++ b/internal/cmd/baremetal_logs_test.go @@ -284,3 +284,20 @@ func (ms *MockSuite) TestBaremetalLogSubscriptionsAreFiltered(assert, require *t assert.Cmp(out, td.Contains("sub-2")) assert.Cmp(out, td.Not(td.Contains("sub-1")), "the subscription the filter excludes must not be printed") } + +// The refusal used to send the operator to `ovhcloud ldp list`, which lists Log +// Data Platform *services*, not Graylog streams — and no command of this tree +// lists streams at all. A remedy that names the wrong thing is the defect this +// whole audit keeps finding, including twice in its own output. +func (ms *MockSuite) TestBaremetalLogsRefusalNamesSomethingThatWorks(assert, require *td.T) { + registerOneLogKind() + registerStreams() + + _, err := cmd.Execute("baremetal", "logs", "subscribe", "ns1.example", + "--stream", "no-such-stream", "--yes") + + require.CmpError(err) + assert.Cmp(err.Error(), td.Not(td.Contains("ovhcloud ldp list")), + "that command lists services, not streams") + assert.Cmp(err.Error(), td.Contains("--stream "), "completion is what actually lists them") +} diff --git a/internal/services/baremetal/logs_stream.go b/internal/services/baremetal/logs_stream.go index 8c5449714..5ec2c7041 100644 --- a/internal/services/baremetal/logs_stream.go +++ b/internal/services/baremetal/logs_stream.go @@ -161,8 +161,12 @@ func resolveStream(wanted string) (ldpStream, error) { return matches[0], nil case 0: + // Not `ovhcloud ldp list`: that lists Log Data Platform services, not + // Graylog streams, and no command of this tree lists streams at all. + // nearbyTitles already says how many there are and which ones are close, + // so pointing at completion is the only thing here that works. return ldpStream{}, fmt.Errorf( - "no stream is called %q. %s\n List them with: ovhcloud ldp list", + "no stream is called %q. %s\n Complete them with: ovhcloud baremetal logs subscribe --stream ", wanted, nearbyTitles(streams, wanted)) default: From 1a1bda3155664c0a6546aaeb3b39b38dc63d2b5d Mon Sep 17 00:00:00 2001 From: Denis Date: Mon, 24 Aug 2026 12:03:02 +0200 Subject: [PATCH 07/11] 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 08/11] 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 cbc1e3cd8911180270db503e75723428b7106b91 Mon Sep 17 00:00:00 2001 From: Denis Date: Mon, 24 Aug 2026 12:10:49 +0200 Subject: [PATCH 09/11] 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 + From 8cce91e7c03043d4cfc47f7754b8b0ab431c040d Mon Sep 17 00:00:00 2001 From: Denis Date: Mon, 24 Aug 2026 18:57:12 +0200 Subject: [PATCH 10/11] fix(logs): name a few streams when none is close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit K3, from the review sandbox. Asking for a stream that matches nothing answered "no stream is called \"prod nginx\". This account has 59 of them" — a count that names nothing, followed by a hint that needs a TAB key a script, a CI log or a web console does not have. nearbyTitles already lists near misses when there are any; the no-near-miss branch now shows three real titles as well, sorted so the same account gives the same examples twice running. An error message that reshuffles itself between two identical calls reads as instability rather than as an example. Signed-off-by: Denis --- internal/services/baremetal/logs_stream.go | 26 +++++++++++++++++++++- internal/services/baremetal/logs_test.go | 22 +++++++++++++++--- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/internal/services/baremetal/logs_stream.go b/internal/services/baremetal/logs_stream.go index 5ec2c7041..f3409e863 100644 --- a/internal/services/baremetal/logs_stream.go +++ b/internal/services/baremetal/logs_stream.go @@ -202,7 +202,14 @@ func nearbyTitles(streams []ldpStream, wanted string) string { } if len(near) == 0 { - return fmt.Sprintf("This account has %d of them.", len(streams)) + // A bare count names nothing, and the reviewer who hit this said so: + // "no stream is called X. This account has 59 of them" leaves you with + // no idea what a title even looks like here, and the completion hint + // that follows needs a TAB key — which a script, a CI log or a web + // console does not have. Three examples cost one line and show the + // shape. + return fmt.Sprintf("This account has %d of them, such as %s.", + len(streams), strings.Join(someTitles(streams, 3), ", ")) } // Five is enough to recognise a typo. The rest are counted rather than @@ -216,6 +223,23 @@ func nearbyTitles(streams []ldpStream, wanted string) string { return "Close to: " + strings.Join(near, ", ") + "." } +// someTitles picks the first few titles that exist, to show what one looks +// like. Sorted, so the same account gives the same examples twice running: an +// error message that changes between two identical calls reads as instability. +func someTitles(streams []ldpStream, count int) []string { + var titles []string + for _, stream := range streams { + if stream.Title != "" { + titles = append(titles, fmt.Sprintf("%q", stream.Title)) + } + } + sort.Strings(titles) + if len(titles) > count { + titles = titles[:count] + } + return titles +} + // looksLikeUUID recognises the shape of a stream identifier. // // It is deliberately a shape test and not a parse: the point is to tell "the diff --git a/internal/services/baremetal/logs_test.go b/internal/services/baremetal/logs_test.go index 1991b25a3..ae3acb352 100644 --- a/internal/services/baremetal/logs_test.go +++ b/internal/services/baremetal/logs_test.go @@ -74,9 +74,25 @@ func TestNearbyTitlesNamesAFewAndCountsTheRest(t *testing.T) { t.Fatalf("five near misses were expected, got %q", near) } - // Nothing close: say how many exist rather than pretend to suggest. - if far := nearbyTitles(streams, "zzzz"); !strings.Contains(far, "7 of them") { - t.Fatalf("got %q", far) + // Nothing close: say how many exist, AND name a few. The reviewer who hit + // this got "no stream is called X. This account has 59 of them" and asked + // the obvious question — which ones? A bare count names nothing, and the + // completion hint that follows it needs a TAB key that a script, a CI log + // or a web console does not have. + far := nearbyTitles(streams, "zzzz") + if !strings.Contains(far, "7 of them") { + t.Fatalf("the count has to survive, got %q", far) + } + if !strings.Contains(far, "such as") { + t.Fatalf("a bare count names nothing: examples are the point, got %q", far) + } + if strings.Count(far, `"`) != 6 { + t.Fatalf("three titles were expected, quoted, got %q", far) + } + // Stable between two identical calls: an error message that reshuffles + // itself reads as instability rather than as an example. + if far != nearbyTitles(streams, "zzzz") { + t.Fatal("the same account has to give the same examples twice running") } } From 401ad618368240a7b407f35e3bb8150b109b9408 Mon Sep 17 00:00:00 2001 From: Denis Date: Wed, 26 Aug 2026 00:01:01 +0200 Subject: [PATCH 11/11] feat(baremetal): say which stream a title resolved to, before sending anything MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--stream "prod nginx"` takes a title because nobody has a stream UUID at hand. The resolution then happened in silence: the confirmation prompt named it, and a dry run never shows the prompt, so the only trace left was a UUID inside the request body. That is the one thing worth checking before this call. Measured on the test account, 2026-08-25: 59 Graylog streams across 25 Log Data Platform services, one service carrying 21 of them, and three titles shared by two streams each ("test_iplb", "test", "Stream1"). Picking the wrong one sends a machine's logs to another service, and it looks like a success. So the dry run now leads with what the title became: 🔍 Dry run: nothing was sent. "Tech_writer_private_cloud" resolves to stream ca06a2f5-… on Log Data Platform service ldp-cg-65382 This would have been called: POST /v2/dedicated/server/{sn}/log/subscription An identifier resolves to itself, so the line does not appear: a note printed on every run is noise, and noise gets skipped exactly when it matters. The success and failure messages move from the bare UUID to streamLabel(), which already carried the title and the service. Tested by sabotage, twice — dropping the line, and dropping the service name from it. Both turn the new tests red. --- internal/cmd/baremetal_logs_test.go | 34 +++++++++++++++++++++++ internal/services/baremetal/logs.go | 43 ++++++++++++++++++++++++----- 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/internal/cmd/baremetal_logs_test.go b/internal/cmd/baremetal_logs_test.go index 93192252c..a334c8029 100644 --- a/internal/cmd/baremetal_logs_test.go +++ b/internal/cmd/baremetal_logs_test.go @@ -159,6 +159,40 @@ func (ms *MockSuite) TestBaremetalLogSubscribeResolvesATitle(assert, require *td assert.Cmp(sent["streamId"], "33333333-3333-3333-3333-333333333333") } +// Resolving a title silently is how a machine's logs end up on the wrong +// service. This account carries 59 streams over 25 Log Data Platform services, +// and three titles are shared by two streams each: the operator has to see +// which one was picked, and on which service, BEFORE the call goes out. +// +// The confirmation prompt said it — and a dry run never shows the prompt, so +// the only trace left was a UUID inside the payload. +func (ms *MockSuite) TestBaremetalLogSubscribeNamesWhatTheTitleResolvedTo(assert, require *td.T) { + registerOneLogKind() + registerStreams() + + out, err := cmd.Execute("baremetal", "logs", "subscribe", "ns1.example", + "--stream", "TO REMOVE 1", "--dry-run") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains("TO REMOVE 1"), "the title asked for") + assert.Cmp(out, td.Contains("33333333-3333-3333-3333-333333333333"), "the stream it became") + assert.Cmp(out, td.Contains("ldp-"), "and the service carrying it") +} + +// The other way round: an identifier resolves to itself, so there is nothing to +// report and the line must not appear. A resolution line on every run would be +// noise, and noise gets skipped exactly when it matters. +func (ms *MockSuite) TestBaremetalLogSubscribeSaysNothingWhenGivenAnIdentifier(assert, require *td.T) { + registerOneLogKind() + registerStreams() + + out, err := cmd.Execute("baremetal", "logs", "subscribe", "ns1.example", + "--stream", "33333333-3333-3333-3333-333333333333", "--dry-run") + + require.CmpNoError(err) + assert.Cmp(out, td.Not(td.Contains("resolves to"))) +} + // The operation says the platform finished. The subscription says the logs // actually go somewhere. Every wait in this CLI reads the second. func (ms *MockSuite) TestBaremetalLogSubscribeWaitReadsTheSubscriptionBack(assert, require *td.T) { diff --git a/internal/services/baremetal/logs.go b/internal/services/baremetal/logs.go index ad51eec33..e1d070d3d 100644 --- a/internal/services/baremetal/logs.go +++ b/internal/services/baremetal/logs.go @@ -135,7 +135,7 @@ func ShowBaremetalLogURL(_ *cobra.Command, args []string) { endpoint := fmt.Sprintf(logURLPath, url.PathEscape(server)) body := map[string]any{"kind": kind} - if reportLogDryRun(http.MethodPost, endpoint, body) { + if reportLogDryRun(http.MethodPost, endpoint, body, "") { return } @@ -214,6 +214,7 @@ func SubscribeBaremetalLogs(_ *cobra.Command, args []string) { return } + // Disruptive rather than destructive: nothing is lost, but the logs of a // machine start landing somewhere new and the indexing they cause is // billed on the receiving service. @@ -226,7 +227,7 @@ func SubscribeBaremetalLogs(_ *cobra.Command, args []string) { endpoint := fmt.Sprintf(logSubscriptionsPath, url.PathEscape(server)) body := map[string]any{"kind": kind, "streamId": stream.StreamID} - if reportLogDryRun(http.MethodPost, endpoint, body) { + if reportLogDryRun(http.MethodPost, endpoint, body, resolutionNote(stream)) { return } @@ -236,14 +237,14 @@ func SubscribeBaremetalLogs(_ *cobra.Command, args []string) { } if err := httpLib.Client.Post(endpoint, body, &response); err != nil { display.OutputError(&flags.OutputFormatConfig, - "failed to subscribe %s to stream %s: %s", server, stream.StreamID, err) + "failed to subscribe %s to stream %s: %s", server, streamLabel(stream), err) return } if !LogWait { display.OutputInfo(&flags.OutputFormatConfig, response, "⚡️ The %s logs of %s are being subscribed to stream %s. Follow it with: ovhcloud baremetal logs subscription list %s", - kind, server, stream.StreamID, server) + kind, server, streamLabel(stream), server) return } @@ -310,7 +311,7 @@ func UnsubscribeBaremetalLogs(_ *cobra.Command, args []string) { return } - if reportLogDryRun(http.MethodDelete, path, nil) { + if reportLogDryRun(http.MethodDelete, path, nil, "") { return } @@ -435,6 +436,23 @@ func waitForLogOperation(ldpService, operationID string) (map[string]any, error) time.Duration(logPollAttempts)*logPollInterval, operationID, ldpService, last) } +// resolutionNote dit ce qu un TITRE a designe : le stream ET le service qui le +// porte. Vide quand l operateur a donne un identifiant -- il n y a alors rien a +// resoudre, et une ligne sur chaque execution serait du bruit, donc sautee +// exactement le jour ou elle compte. +// +// Mesure du 25/08 sur le compte de test : 59 streams sur 25 services, et trois +// titres portes par deux streams chacun. Sans cette ligne, la seule trace de la +// resolution etait un UUID dans le corps de la requete. +func resolutionNote(stream ldpStream) string { + if stream.Title == "" { + return "" + } + + return fmt.Sprintf(" %q resolves to stream %s on Log Data Platform service %s", + stream.Title, stream.StreamID, stream.ServiceName) +} + // streamLabel names a stream the way the operator asked for it. // // When a title was resolved, the title is what they recognise and the @@ -476,15 +494,26 @@ func expiryPhrase(expiration string) string { // neither is in the URL. Printing them as a second message would mean two JSON // documents on one stdout under -o json, so this builds the one document // itself — the same reason `baremetal ticket` does. -func reportLogDryRun(method, endpoint string, body map[string]any) bool { +// reportLogDryRun rend true quand la commande s arrete la. `note` dit ce que +// l operateur ne pourrait pas deduire du corps -- aujourd hui, sur quel stream +// et quel service un TITRE est tombe. Le corps ne porte qu un UUID, et la +// question de confirmation, qui le disait, ne s affiche pas en dry-run. +func reportLogDryRun(method, endpoint string, body map[string]any, note string) bool { if !flags.DryRun { return false } - message := fmt.Sprintf("🔍 Dry run: nothing was sent. This would have been called:\n %s %s", method, endpoint) + message := "🔍 Dry run: nothing was sent." details := map[string]any{ "calls": []map[string]any{{"method": method, "endpoint": endpoint}}, } + // La resolution vient avant l appel : elle est le contexte de ce qui suit, + // et c est la ligne sur laquelle on renonce ou on continue. + if note != "" { + message += "\n" + note + details["resolved"] = strings.TrimSpace(note) + } + message += fmt.Sprintf("\nThis would have been called:\n %s %s", method, endpoint) if len(body) > 0 { rendered, err := json.MarshalIndent(body, " ", " ")