[OSD-21620] Add validation for the backplane config file

cmd/ocm-backplane/console/console.go

@@ -34,10 +34,6 @@ import (
 
  "github.com/Masterminds/semver"
  homedir "github.com/mitchellh/go-homedir"
- "github.com/openshift/backplane-cli/pkg/cli/config"
- "github.com/openshift/backplane-cli/pkg/info"
- "github.com/openshift/backplane-cli/pkg/ocm"
- "github.com/openshift/backplane-cli/pkg/utils"
  consolev1typedclient "github.com/openshift/client-go/console/clientset/versioned/typed/console/v1"
  consolev1alpha1typedclient "github.com/openshift/client-go/console/clientset/versioned/typed/console/v1alpha1"
  operatorv1typedclient "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1"
@@ -49,6 +45,11 @@ import (
  "k8s.io/cli-runtime/pkg/genericclioptions"
  "k8s.io/client-go/kubernetes"
  "k8s.io/client-go/rest"
+
+ "github.com/openshift/backplane-cli/pkg/cli/config"
+ "github.com/openshift/backplane-cli/pkg/info"
+ "github.com/openshift/backplane-cli/pkg/ocm"
+ "github.com/openshift/backplane-cli/pkg/utils"
 )
 
 type containerEngineInterface interface {
@@ -1194,7 +1195,7 @@ func dockerRunMonitorPlugin(containerName string, consoleContainerName string, n
 }
 
 func (ce *dockerLinux) runMonitorPlugin(containerName string, consoleContainerName string, nginxConf string, pluginArgs []string) error {
- configDirectory, err := config.GetConfigDirctory()
+ configDirectory, err := config.GetConfigDirectory()
  if err != nil {
  return err
  }
@@ -1203,7 +1204,7 @@ func (ce *dockerLinux) runMonitorPlugin(containerName string, consoleContainerNa
 }
 
 func (ce *dockerMac) runMonitorPlugin(containerName string, consoleContainerName string, nginxConf string, pluginArgs []string) error {
- configDirectory, err := config.GetConfigDirctory()
+ configDirectory, err := config.GetConfigDirectory()
  if err != nil {
  return err
  }
@@ -1265,7 +1266,7 @@ func (ce *podmanMac) putFileToMount(filename string, content []byte) error {
 // filename should be name only, not a path
 func dockerPutFileToMount(filename string, content []byte) error {
  // for files in linux, we put them into the user's backplane config directory
- configDirectory, err := config.GetConfigDirctory()
+ configDirectory, err := config.GetConfigDirectory()
  if err != nil {
  return err
  }

cmd/ocm-backplane/login/login.go

@@ -122,6 +122,9 @@ func runLogin(cmd *cobra.Command, argv []string) (err error) {
  logger.Debugf("Extracting Backplane Cluster ID")
  // Currently go-pagerduty pkg does not include incident id validation.
  if args.pd != "" {
+ if bpConfig.PagerDutyAPIKey == "" {
+ return fmt.Errorf("please make sure the PD API Key is configured correctly in the config file")
+ }
  pdClient, err := pagerduty.NewWithToken(bpConfig.PagerDutyAPIKey)
  if err != nil {
  return fmt.Errorf("could not initialize the client: %w", err)

cmd/ocm-backplane/login/login_test.go

@@ -14,6 +14,9 @@ import (
  . "github.com/onsi/ginkgo"
  . "github.com/onsi/gomega"
  cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
+ "k8s.io/client-go/tools/clientcmd"
+ "k8s.io/client-go/tools/clientcmd/api"
+
  "github.com/openshift/backplane-cli/pkg/backplaneapi"
  backplaneapiMock "github.com/openshift/backplane-cli/pkg/backplaneapi/mocks"
  "github.com/openshift/backplane-cli/pkg/cli/config"
@@ -22,8 +25,6 @@ import (
  "github.com/openshift/backplane-cli/pkg/ocm"
  ocmMock "github.com/openshift/backplane-cli/pkg/ocm/mocks"
  "github.com/openshift/backplane-cli/pkg/utils"
- "k8s.io/client-go/tools/clientcmd"
- "k8s.io/client-go/tools/clientcmd/api"
 )
 
 func MakeIoReader(s string) io.ReadCloser {
@@ -464,6 +465,43 @@ var _ = Describe("Login command", func() {
  Expect(err.Error()).Should(ContainSubstring("status code 401"))
  })
 
+ It("should return error when trying to login via PD but the PD API Key is not configured", func() {
+ args.pd = truePagerDutyIncidentID
+
+ err := utils.CreateTempKubeConfig(nil)
+ Expect(err).To(BeNil())
+
+ mockOcmInterface.EXPECT().GetOCMEnvironment().Return(ocmEnv, nil).AnyTimes()
+
+ // Create a temporary JSON configuration file in the temp directory for testing purposes.
+ tempDir := os.TempDir()
+ bpConfigPath = filepath.Join(tempDir, "mock.json")
+ tempFile, err := os.Create(bpConfigPath)
+ Expect(err).To(BeNil())
+
+ testData := config.BackplaneConfiguration{
+ URL: backplaneAPIURI,
+ ProxyURL: new(string),
+ SessionDirectory: "",
+ AssumeInitialArn: "",
+ PagerDutyAPIKey: falsePagerDutyAPITkn,
+ }
+
+ // Marshal the testData into JSON format and write it to tempFile.
+ pdTestData := testData
+ pdTestData.PagerDutyAPIKey = ""
+ jsonData, err := json.Marshal(pdTestData)
+ Expect(err).To(BeNil())
+ _, err = tempFile.Write(jsonData)
+ Expect(err).To(BeNil())
+
+ os.Setenv("BACKPLANE_CONFIG", bpConfigPath)
+
+ err = runLogin(nil, nil)
+
+ Expect(err.Error()).Should(ContainSubstring("please make sure the PD API Key is configured correctly"))
+ })
+
  It("should fail to find a non existent PD Incident and return HTTP status code 404 when the requested resource is not found", func() {
  args.pd = falsePagerDutyIncidentID
 

cmd/ocm-backplane/logout/logout_test.go

@@ -14,6 +14,7 @@ import (
  "k8s.io/client-go/tools/clientcmd/api"
 
  cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
+
  "github.com/openshift/backplane-cli/cmd/ocm-backplane/login"
  "github.com/openshift/backplane-cli/pkg/backplaneapi"
  backplaneapiMock "github.com/openshift/backplane-cli/pkg/backplaneapi/mocks"

pkg/cli/config/config.go

@@ -15,9 +15,10 @@ import (
  "github.com/openshift/backplane-cli/pkg/ocm"
 )
 
+// Please update the validateConfig function if there is any required config key added
 type BackplaneConfiguration struct {
  URL string `json:"url"`
- ProxyURL *string `json:"proxy-url"` // Optional
+ ProxyURL *string `json:"proxy-url"`
  SessionDirectory string `json:"session-dir"`
  AssumeInitialArn string `json:"assume-initial-arn"`
  PagerDutyAPIKey string `json:"pd-key"`
@@ -61,6 +62,11 @@ func GetBackplaneConfiguration() (bpConfig BackplaneConfiguration, err error) {
  }
  }
 
+ if err = validateConfig(); err != nil {
+ // FIXME: we should return instead of warning here, after the tests do not require external network access
+ logger.Warn(err)
+ }
+
  // Check if user has explicitly defined proxy; it has higher precedence over the config file
  err = viper.BindEnv("proxy-url", info.BackplaneProxyEnvName)
  if err != nil {
@@ -72,9 +78,10 @@ func GetBackplaneConfiguration() (bpConfig BackplaneConfiguration, err error) {
  logger.Warn("Manual URL configuration is deprecated, please remove URL key from Backplane configuration")
  }
 
- // Check if user has explicitly defined backplane URL via env; it has higher precedence over the ocm env URL
+ // Warn if user has explicitly defined backplane URL via env
  url, ok := getBackplaneEnv(info.BackplaneURLEnvName)
  if ok {
+ logger.Warn(fmt.Printf("Manual URL configuration is deprecated, please unset the environment %s", info.BackplaneURLEnvName))
  bpConfig.URL = url
  } else {
  // Fetch backplane URL from ocm env
@@ -88,8 +95,6 @@ func GetBackplaneConfiguration() (bpConfig BackplaneConfiguration, err error) {
  proxyURL := bpConfig.getFirstWorkingProxyURL(proxyInConfigFile)
  if proxyURL != "" {
  bpConfig.ProxyURL = &proxyURL
- } else {
- logger.Warn("No proxy configuration available. This may result in failing commands as backplane-api is only available from select networks.")
  }
 
  bpConfig.SessionDirectory = viper.GetString("session-dir")
@@ -102,7 +107,6 @@ func GetBackplaneConfiguration() (bpConfig BackplaneConfiguration, err error) {
  } else {
  logger.Info("No PagerDuty API Key configuration available. This will result in failure of `ocm-backplane login --pd <incident-id>` command.")
  }
-
  return bpConfig, nil
 }
 
@@ -145,7 +149,18 @@ func (config *BackplaneConfiguration) getFirstWorkingProxyURL(s []string) string
  return ""
 }
 
-func GetConfigDirctory() (string, error) {
+func validateConfig() error {
+
+ // Validate the proxy url
+ if viper.GetStringSlice("proxy-url") == nil && os.Getenv(info.BackplaneProxyEnvName) == "" {
+ return fmt.Errorf("proxy-url must be set explicitly in either config file or via the environment HTTPS_PROXY")
+ }
+
+ return nil
+}
+
+// GetConfigDirectory returns the backplane config path
+func GetConfigDirectory() (string, error) {
  bpConfigFilePath, err := GetConfigFilePath()
  if err != nil {
  return "", err

pkg/cli/config/config_test.go

@@ -3,7 +3,12 @@ package config
 import (
  "net/http"
  "net/http/httptest"
+ "os"
  "testing"
+
+ "github.com/spf13/viper"
+
+ "github.com/openshift/backplane-cli/pkg/info"
 )
 
 func TestGetBackplaneConfig(t *testing.T) {
@@ -25,7 +30,6 @@ func TestGetBackplaneConfig(t *testing.T) {
  t.Errorf("expected to return the explicitly defined proxy %v instead of the default one %v", userDefinedProxy, config.ProxyURL)
  }
  })
-
 }
 
 func TestGetBackplaneConnection(t *testing.T) {
@@ -121,3 +125,36 @@ func TestBackplaneConfiguration_getFirstWorkingProxyURL(t *testing.T) {
  })
  }
 }
+
+func TestValidateConfig(t *testing.T) {
+ tests := []struct {
+ name string
+ proxyConfig []string
+ envProxy string
+ expectError bool
+ }{
+ {"No proxy set", nil, "", true},
+ {"Proxy set in config", []string{"http://proxy.example.com"}, "", false},
+ {"Proxy set in environment", nil, "http://proxy.example.com", false},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ // Set up viper configuration
+ viper.Set("proxy-url", tt.proxyConfig)
+
+ // Set up environment variable
+ if tt.envProxy != "" {
+ os.Setenv(info.BackplaneProxyEnvName, tt.envProxy)
+ } else {
+ os.Unsetenv(info.BackplaneProxyEnvName)
+ }
+
+ // Validate config
+ err := validateConfig()
+ if (err != nil) != tt.expectError {
+ t.Errorf("expected error: %v, got: %v", tt.expectError, err)
+ }
+ })
+ }
+}