From 6dda16fa9ecc7016e235ae6d4b2dbd0ed52b05d6 Mon Sep 17 00:00:00 2001 From: pandemicsyn Date: Wed, 21 Aug 2024 13:07:33 -0500 Subject: [PATCH 1/6] Improved messaging when requested channel slug not allowed by license --- cmd/kots/cli/install.go | 5 +++++ cmd/kots/cli/pull.go | 11 +++++++++-- cmd/kots/cli/util.go | 2 +- cmd/kots/cli/util_test.go | 2 +- pkg/license/multichannel.go | 25 +++++++++++++++++++++++-- 5 files changed, 39 insertions(+), 6 deletions(-) diff --git a/cmd/kots/cli/install.go b/cmd/kots/cli/install.go index 5b8f7a1e5b..f198b7b833 100644 --- a/cmd/kots/cli/install.go +++ b/cmd/kots/cli/install.go @@ -167,6 +167,11 @@ func InstallCmd() *cobra.Command { if err != nil { return errors.Wrap(err, "failed to extract preferred channel slug") } + if preferredChannelSlug == "" { + preferredChannelSlug = "stable" + log.ActionWithoutSpinner(fmt.Sprintf("No channel specified in upstream URI, falling back to channel %q.", preferredChannelSlug)) + } + license, err = kotslicense.VerifyAndUpdateLicense(log, license, preferredChannelSlug, isAirgap) if err != nil { return errors.Wrap(err, "failed to verify and update license") diff --git a/cmd/kots/cli/pull.go b/cmd/kots/cli/pull.go index d4dbdd0ba5..8a262964ed 100644 --- a/cmd/kots/cli/pull.go +++ b/cmd/kots/cli/pull.go @@ -1,6 +1,7 @@ package cli import ( + "fmt" "os" "path" "strings" @@ -101,13 +102,19 @@ func PullCmd() *cobra.Command { } upstream := pull.RewriteUpstream(args[0]) + + log := logger.NewCLILogger(cmd.OutOrStdout()) + log.Initialize() + preferredChannelSlug, err := extractPreferredChannelSlug(upstream) if err != nil { return errors.Wrap(err, "failed to extract preferred channel slug") } - log := logger.NewCLILogger(cmd.OutOrStdout()) - log.Initialize() + if preferredChannelSlug == "" { + preferredChannelSlug = "stable" + log.ActionWithoutSpinner(fmt.Sprintf("No channel specified in upstream URI, falling back to channel %q.", preferredChannelSlug)) + } // If we are passed a multi-channel license, verify that the requested channel is in the license // so that we can warn the user immediately if it is not. diff --git a/cmd/kots/cli/util.go b/cmd/kots/cli/util.go index 4d4ea33f56..41153d4dd3 100644 --- a/cmd/kots/cli/util.go +++ b/cmd/kots/cli/util.go @@ -67,5 +67,5 @@ func extractPreferredChannelSlug(upstreamURI string) (string, error) { if replicatedUpstream.Channel != nil { return *replicatedUpstream.Channel, nil } - return "stable", nil + return "", nil } diff --git a/cmd/kots/cli/util_test.go b/cmd/kots/cli/util_test.go index a270dbaecf..dcffcb6747 100644 --- a/cmd/kots/cli/util_test.go +++ b/cmd/kots/cli/util_test.go @@ -111,7 +111,7 @@ func Test_extractPreferredChannelSlug(t *testing.T) { args{ upstreamURI: "replicated://app-slug", }, - "stable", // default channel + "", false, }, { diff --git a/pkg/license/multichannel.go b/pkg/license/multichannel.go index db7f58d444..f872a990b2 100644 --- a/pkg/license/multichannel.go +++ b/pkg/license/multichannel.go @@ -1,6 +1,9 @@ package license import ( + "fmt" + "strings" + "github.com/pkg/errors" "github.com/replicatedhq/kots/pkg/logger" "github.com/replicatedhq/kots/pkg/replicatedapp" @@ -40,11 +43,20 @@ func VerifyAndUpdateLicense(log *logger.CLILogger, license *kotsv1beta1.License, return nil, nil } if isAirgap { + log.ActionWithSpinner("Verifying channel slug %q allowed by license", preferredChannelSlug) if !canInstallFromChannel(preferredChannelSlug, license) { - return nil, errors.New("requested channel not found in supplied license") + log.FinishSpinnerWithError() + return license, errors.New(fmt.Sprintf("channel slug %q is not allowed by license", preferredChannelSlug)) + } + log.FinishSpinner() + validChannels := []string{} + for _, channel := range license.Spec.Channels { + validChannels = append(validChannels, fmt.Sprintf("%s/%s", license.Spec.AppSlug, channel.ChannelSlug)) } + log.ChildActionWithoutSpinner(fmt.Sprintf("To install from an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) return license, nil } + log.ActionWithSpinner("Checking for license update") // we fetch the latest license to ensure that the license is up to date, before proceeding updatedLicense, err := replicatedapp.GetLatestLicense(license, "") @@ -53,8 +65,17 @@ func VerifyAndUpdateLicense(log *logger.CLILogger, license *kotsv1beta1.License, return nil, errors.Wrap(err, "failed to get latest license") } log.FinishSpinner() + + log.ActionWithSpinner("Verifying channel slug %q allowed by license", preferredChannelSlug) if canInstallFromChannel(preferredChannelSlug, updatedLicense.License) { + log.FinishSpinner() return updatedLicense.License, nil } - return nil, errors.New("requested channel not found in latest license") + log.FinishSpinnerWithError() + validChannels := []string{} + for _, channel := range license.Spec.Channels { + validChannels = append(validChannels, fmt.Sprintf("%s/%s", license.Spec.AppSlug, channel.ChannelSlug)) + } + log.ChildActionWithoutSpinner(fmt.Sprintf("To install from an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) + return updatedLicense.License, errors.New(fmt.Sprintf("channel slug %q is not allowed by latest license", preferredChannelSlug)) } From f473264f652c6e7cf44b4a321af5ebbaa378c3e0 Mon Sep 17 00:00:00 2001 From: pandemicsyn Date: Wed, 21 Aug 2024 13:28:23 -0500 Subject: [PATCH 2/6] make wording install/pull agnostic --- pkg/license/multichannel.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/license/multichannel.go b/pkg/license/multichannel.go index f872a990b2..d4b6ae3960 100644 --- a/pkg/license/multichannel.go +++ b/pkg/license/multichannel.go @@ -53,7 +53,7 @@ func VerifyAndUpdateLicense(log *logger.CLILogger, license *kotsv1beta1.License, for _, channel := range license.Spec.Channels { validChannels = append(validChannels, fmt.Sprintf("%s/%s", license.Spec.AppSlug, channel.ChannelSlug)) } - log.ChildActionWithoutSpinner(fmt.Sprintf("To install from an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) + log.ChildActionWithoutSpinner(fmt.Sprintf("To install/pull an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) return license, nil } @@ -76,6 +76,6 @@ func VerifyAndUpdateLicense(log *logger.CLILogger, license *kotsv1beta1.License, for _, channel := range license.Spec.Channels { validChannels = append(validChannels, fmt.Sprintf("%s/%s", license.Spec.AppSlug, channel.ChannelSlug)) } - log.ChildActionWithoutSpinner(fmt.Sprintf("To install from an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) + log.ChildActionWithoutSpinner(fmt.Sprintf("To install/pull an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) return updatedLicense.License, errors.New(fmt.Sprintf("channel slug %q is not allowed by latest license", preferredChannelSlug)) } From 03cec6376bc63b9b41fe0ef2a0d065b15c16c8d8 Mon Sep 17 00:00:00 2001 From: Florian Hines Date: Thu, 22 Aug 2024 00:01:18 +0000 Subject: [PATCH 3/6] pr feedback --- cmd/kots/cli/install.go | 6 +----- cmd/kots/cli/pull.go | 8 +------- cmd/kots/cli/util.go | 9 +++++++-- cmd/kots/cli/util_test.go | 4 ++-- 4 files changed, 11 insertions(+), 16 deletions(-) diff --git a/cmd/kots/cli/install.go b/cmd/kots/cli/install.go index f198b7b833..7d700e7ff7 100644 --- a/cmd/kots/cli/install.go +++ b/cmd/kots/cli/install.go @@ -163,14 +163,10 @@ func InstallCmd() *cobra.Command { }() upstream := pull.RewriteUpstream(args[0]) - preferredChannelSlug, err := extractPreferredChannelSlug(upstream) + preferredChannelSlug, err := extractPreferredChannelSlug(log, upstream) if err != nil { return errors.Wrap(err, "failed to extract preferred channel slug") } - if preferredChannelSlug == "" { - preferredChannelSlug = "stable" - log.ActionWithoutSpinner(fmt.Sprintf("No channel specified in upstream URI, falling back to channel %q.", preferredChannelSlug)) - } license, err = kotslicense.VerifyAndUpdateLicense(log, license, preferredChannelSlug, isAirgap) if err != nil { diff --git a/cmd/kots/cli/pull.go b/cmd/kots/cli/pull.go index 8a262964ed..25ad78b4e2 100644 --- a/cmd/kots/cli/pull.go +++ b/cmd/kots/cli/pull.go @@ -1,7 +1,6 @@ package cli import ( - "fmt" "os" "path" "strings" @@ -106,16 +105,11 @@ func PullCmd() *cobra.Command { log := logger.NewCLILogger(cmd.OutOrStdout()) log.Initialize() - preferredChannelSlug, err := extractPreferredChannelSlug(upstream) + preferredChannelSlug, err := extractPreferredChannelSlug(log, upstream) if err != nil { return errors.Wrap(err, "failed to extract preferred channel slug") } - if preferredChannelSlug == "" { - preferredChannelSlug = "stable" - log.ActionWithoutSpinner(fmt.Sprintf("No channel specified in upstream URI, falling back to channel %q.", preferredChannelSlug)) - } - // If we are passed a multi-channel license, verify that the requested channel is in the license // so that we can warn the user immediately if it is not. license, err = kotslicense.VerifyAndUpdateLicense(log, license, preferredChannelSlug, false) diff --git a/cmd/kots/cli/util.go b/cmd/kots/cli/util.go index 41153d4dd3..3a461ff9a8 100644 --- a/cmd/kots/cli/util.go +++ b/cmd/kots/cli/util.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/pkg/errors" + "github.com/replicatedhq/kots/pkg/logger" "github.com/replicatedhq/kots/pkg/replicatedapp" "github.com/replicatedhq/kots/pkg/util" ) @@ -53,7 +54,7 @@ func splitEndpointAndNamespace(endpoint string) (string, string) { return registryEndpoint, registryNamespace } -func extractPreferredChannelSlug(upstreamURI string) (string, error) { +func extractPreferredChannelSlug(log *logger.CLILogger, upstreamURI string) (string, error) { u, err := url.ParseRequestURI(upstreamURI) if err != nil { return "", errors.Wrap(err, "failed to parse uri") @@ -67,5 +68,9 @@ func extractPreferredChannelSlug(upstreamURI string) (string, error) { if replicatedUpstream.Channel != nil { return *replicatedUpstream.Channel, nil } - return "", nil + + if log != nil { + log.ActionWithoutSpinner("No channel specified in upstream URI, falling back to channel slug 'stable.") + } + return "stable", nil } diff --git a/cmd/kots/cli/util_test.go b/cmd/kots/cli/util_test.go index dcffcb6747..50262ae0f9 100644 --- a/cmd/kots/cli/util_test.go +++ b/cmd/kots/cli/util_test.go @@ -111,7 +111,7 @@ func Test_extractPreferredChannelSlug(t *testing.T) { args{ upstreamURI: "replicated://app-slug", }, - "", + "stable", false, }, { @@ -133,7 +133,7 @@ func Test_extractPreferredChannelSlug(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := extractPreferredChannelSlug(tt.args.upstreamURI) + got, err := extractPreferredChannelSlug(nil, tt.args.upstreamURI) if (err != nil) != tt.wantErr { t.Errorf("extractPreferredChannelSlug() error = %v, wantErr %v", err, tt.wantErr) return From ea4189e113b0726f8075f23a8295b1243a77553a Mon Sep 17 00:00:00 2001 From: Florian Hines Date: Fri, 23 Aug 2024 13:52:40 -0500 Subject: [PATCH 4/6] Update cmd/kots/cli/util.go Co-authored-by: Salah Al Saleh --- cmd/kots/cli/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/kots/cli/util.go b/cmd/kots/cli/util.go index 3a461ff9a8..ac22cdec04 100644 --- a/cmd/kots/cli/util.go +++ b/cmd/kots/cli/util.go @@ -70,7 +70,7 @@ func extractPreferredChannelSlug(log *logger.CLILogger, upstreamURI string) (str } if log != nil { - log.ActionWithoutSpinner("No channel specified in upstream URI, falling back to channel slug 'stable.") + log.ActionWithoutSpinner("No channel specified in upstream URI, falling back to channel slug 'stable'.") } return "stable", nil } From d6eb299b70b5a2fa36438a15016760f09f29e23b Mon Sep 17 00:00:00 2001 From: Florian Hines Date: Fri, 23 Aug 2024 13:53:20 -0500 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Salah Al Saleh --- pkg/license/multichannel.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/license/multichannel.go b/pkg/license/multichannel.go index d4b6ae3960..4f7470bccf 100644 --- a/pkg/license/multichannel.go +++ b/pkg/license/multichannel.go @@ -43,7 +43,7 @@ func VerifyAndUpdateLicense(log *logger.CLILogger, license *kotsv1beta1.License, return nil, nil } if isAirgap { - log.ActionWithSpinner("Verifying channel slug %q allowed by license", preferredChannelSlug) + log.ActionWithSpinner("Verifying if channel slug %q is allowed by the license", preferredChannelSlug) if !canInstallFromChannel(preferredChannelSlug, license) { log.FinishSpinnerWithError() return license, errors.New(fmt.Sprintf("channel slug %q is not allowed by license", preferredChannelSlug)) @@ -53,7 +53,7 @@ func VerifyAndUpdateLicense(log *logger.CLILogger, license *kotsv1beta1.License, for _, channel := range license.Spec.Channels { validChannels = append(validChannels, fmt.Sprintf("%s/%s", license.Spec.AppSlug, channel.ChannelSlug)) } - log.ChildActionWithoutSpinner(fmt.Sprintf("To install/pull an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) + log.ChildActionWithoutSpinner(fmt.Sprintf("To install an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) return license, nil } @@ -66,7 +66,7 @@ func VerifyAndUpdateLicense(log *logger.CLILogger, license *kotsv1beta1.License, } log.FinishSpinner() - log.ActionWithSpinner("Verifying channel slug %q allowed by license", preferredChannelSlug) + log.ActionWithSpinner("Verifying if channel slug %q is allowed by the license", preferredChannelSlug) if canInstallFromChannel(preferredChannelSlug, updatedLicense.License) { log.FinishSpinner() return updatedLicense.License, nil @@ -76,6 +76,6 @@ func VerifyAndUpdateLicense(log *logger.CLILogger, license *kotsv1beta1.License, for _, channel := range license.Spec.Channels { validChannels = append(validChannels, fmt.Sprintf("%s/%s", license.Spec.AppSlug, channel.ChannelSlug)) } - log.ChildActionWithoutSpinner(fmt.Sprintf("To install/pull an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) + log.ChildActionWithoutSpinner(fmt.Sprintf("To install an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) return updatedLicense.License, errors.New(fmt.Sprintf("channel slug %q is not allowed by latest license", preferredChannelSlug)) } From 7822527346b730ce3c21c33b9201c029bf365c7f Mon Sep 17 00:00:00 2001 From: pandemicsyn Date: Wed, 4 Sep 2024 13:45:10 -0500 Subject: [PATCH 6/6] pr feedback --- pkg/license/multichannel.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/pkg/license/multichannel.go b/pkg/license/multichannel.go index 4f7470bccf..daad55f299 100644 --- a/pkg/license/multichannel.go +++ b/pkg/license/multichannel.go @@ -43,18 +43,15 @@ func VerifyAndUpdateLicense(log *logger.CLILogger, license *kotsv1beta1.License, return nil, nil } if isAirgap { - log.ActionWithSpinner("Verifying if channel slug %q is allowed by the license", preferredChannelSlug) - if !canInstallFromChannel(preferredChannelSlug, license) { - log.FinishSpinnerWithError() - return license, errors.New(fmt.Sprintf("channel slug %q is not allowed by license", preferredChannelSlug)) + if canInstallFromChannel(preferredChannelSlug, license) { + return license, nil } - log.FinishSpinner() validChannels := []string{} for _, channel := range license.Spec.Channels { validChannels = append(validChannels, fmt.Sprintf("%s/%s", license.Spec.AppSlug, channel.ChannelSlug)) } - log.ChildActionWithoutSpinner(fmt.Sprintf("To install an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) - return license, nil + log.Errorf("Channel slug %q is not allowed by license. Please use one of the following: %s", preferredChannelSlug, strings.Join(validChannels, ", ")) + return license, errors.New(fmt.Sprintf("channel slug %q is not allowed by license", preferredChannelSlug)) } log.ActionWithSpinner("Checking for license update") @@ -66,16 +63,13 @@ func VerifyAndUpdateLicense(log *logger.CLILogger, license *kotsv1beta1.License, } log.FinishSpinner() - log.ActionWithSpinner("Verifying if channel slug %q is allowed by the license", preferredChannelSlug) if canInstallFromChannel(preferredChannelSlug, updatedLicense.License) { - log.FinishSpinner() return updatedLicense.License, nil } - log.FinishSpinnerWithError() validChannels := []string{} for _, channel := range license.Spec.Channels { validChannels = append(validChannels, fmt.Sprintf("%s/%s", license.Spec.AppSlug, channel.ChannelSlug)) } - log.ChildActionWithoutSpinner(fmt.Sprintf("To install an allowed channel, use one of the following: %s", strings.Join(validChannels, ", "))) + log.Errorf("Channel slug %q is not allowed by license. Please use one of the following: %s", preferredChannelSlug, strings.Join(validChannels, ", ")) return updatedLicense.License, errors.New(fmt.Sprintf("channel slug %q is not allowed by latest license", preferredChannelSlug)) }