[release-4.13] OCPBUGS-22914: remove username & password config options (openshift#859)

* OCPBUGS-21797: remove username & password config options

* review update

---------

Co-authored-by: Tomáš Remeš <[email protected]>

Author: openshift-cherrypick-robot

docs/arch.md

@@ -10,7 +10,6 @@ The `support` secret provides following configuration attributes:
 - `endpoint` - upload endpoint - default is `https://console.redhat.com/api/ingress/v1/upload`,
 - `interval` - data gathering & uploading frequency - default is `2h`
 - `httpProxy`, `httpsProxy`, `noProxy` eventually to set custom proxy, which overrides cluster proxy just for the Insights Operator
-- `username`, `password` - if set, the insights client upload will be authenticated by basic authorization using the username/password. By default, it uses the token (see below) from the `pull-secret` secret.
 - `enableGlobalObfuscation` - to enable the global obfuscation of the IP addresses and the cluster domain name. Default value is `false`
 - `reportEndpoint` - download endpoint. From this endpoint, the Insights operator downloads the latest Insights analysis. Default value is `https://console.redhat.com/api/insights-results-aggregator/v2/cluster/%s/reports` (where `%s` must be replaced with the cluster ID)
 - `reportPullingDelay` - the delay between data upload and download. Default value is `60s`

pkg/authorizer/clusterauthorizer/clusterauthorizer.go

@@ -28,17 +28,10 @@ func New(configurator configobserver.Configurator) *Authorizer {
 
 // Authorize adds the necessary auth header to the request, depending on the config. (BasicAuth/Token)
 func (a *Authorizer) Authorize(req *http.Request) error {
-	cfg := a.configurator.Config()
-
 	if req.Header == nil {
 		req.Header = make(http.Header)
 	}
 
-	if len(cfg.Username) > 0 || len(cfg.Password) > 0 {
-		req.SetBasicAuth(cfg.Username, cfg.Password)
-		return nil
-	}
-
 	token, err := a.Token()
 	if err != nil {
 		return err

pkg/config/config.go

@@ -50,9 +50,7 @@ type Controller struct {
 	// To see the detailed info about how anonymization works, go to the docs of package anonymization.
 	EnableGlobalObfuscation bool
 
-	Username string
-	Password string
-	Token    string
+	Token string
 
 	HTTPConfig HTTPConfig
 	OCMConfig  OCMConfig
@@ -85,7 +83,6 @@ func (c *Controller) ToString() string {
 		"endpoint=%s "+
 		"conditional_gatherer_endpoint=%s "+
 		"interval=%s "+
-		"username=%t "+
 		"token=%t "+
 		"reportEndpoint=%s "+
 		"initialPollingDelay=%s "+
@@ -95,7 +92,6 @@ func (c *Controller) ToString() string {
 		c.Endpoint,
 		c.ConditionalGathererEndpoint,
 		c.Interval,
-		len(c.Username) > 0,
 		len(c.Token) > 0,
 		c.ReportEndpoint,
 		c.ReportPullingDelay,
@@ -104,7 +100,6 @@ func (c *Controller) ToString() string {
 }
 
 func (c *Controller) MergeWith(cfg *Controller) {
-	c.mergeCredentials(cfg)
 	c.mergeInterval(cfg)
 	c.mergeEndpoint(cfg)
 	c.mergeConditionalGathererEndpoint(cfg)
@@ -113,11 +108,6 @@ func (c *Controller) MergeWith(cfg *Controller) {
 	c.mergeHTTP(cfg)
 }
 
-func (c *Controller) mergeCredentials(cfg *Controller) {
-	c.Username = cfg.Username
-	c.Password = cfg.Password
-}
-
 func (c *Controller) mergeEndpoint(cfg *Controller) {
 	if len(cfg.Endpoint) > 0 {
 		c.Endpoint = cfg.Endpoint

pkg/config/configobserver/config.go

@@ -24,7 +24,6 @@ func LoadConfigFromSecret(secret *v1.Secret) (config.Controller, error) {
 	var cfg Config
 	var err error
 
-	cfg.loadCredentials(secret.Data)
 	cfg.loadEndpoint(secret.Data)
 	cfg.loadConditionalGathererEndpoint(secret.Data)
 	cfg.loadHTTP(secret.Data)
@@ -50,15 +49,6 @@ func LoadConfigFromSecret(secret *v1.Secret) (config.Controller, error) {
 	return cfg.Controller, err
 }
 
-func (c *Config) loadCredentials(data map[string][]byte) {
-	if username, ok := data["username"]; ok {
-		c.Username = strings.TrimSpace(string(username))
-	}
-	if password, ok := data["password"]; ok {
-		c.Password = strings.TrimSpace(string(password))
-	}
-}
-
 func (c *Config) loadEndpoint(data map[string][]byte) {
 	if endpoint, ok := data["endpoint"]; ok {
 		c.Endpoint = string(endpoint)

pkg/config/configobserver/config_test.go

@@ -10,36 +10,6 @@ import (
 	"github.com/openshift/insights-operator/pkg/config"
 )
 
-func TestConfig_loadCredentials(t *testing.T) {
-	tests := []struct {
-		name string
-		data map[string][]byte
-		want *Config
-	}{
-		{
-			name: "Load credentials",
-			data: map[string][]byte{
-				"username": []byte("user"),
-				"password": []byte("xxxxxx"),
-			},
-			want: &Config{Controller: config.Controller{
-				Report:   false,
-				Username: "user",
-				Password: "xxxxxx",
-			}},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := &Config{Controller: config.Controller{}}
-			got.loadCredentials(tt.data)
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("loadCredentials() got = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 func TestConfig_loadEndpoint(t *testing.T) {
 	tests := []struct {
 		name string
@@ -194,8 +164,6 @@ func TestLoadConfigFromSecret(t *testing.T) {
 			name: "Can load from secret",
 			secret: &v1.Secret{
 				Data: map[string][]byte{
-					"username":        []byte("user"),
-					"password":        []byte("xxxxxx"),
 					"endpoint":        []byte("http://endpoint"),
 					"noProxy":         []byte("no-proxy"),
 					"reportEndpoint":  []byte("http://report"),
@@ -206,8 +174,6 @@ func TestLoadConfigFromSecret(t *testing.T) {
 				Report:             true,
 				Endpoint:           "http://endpoint",
 				ReportEndpoint:     "http://report",
-				Username:           "user",
-				Password:           "xxxxxx",
 				ReportPullingDelay: time.Duration(-1),
 				HTTPConfig: config.HTTPConfig{
 					NoProxy: "no-proxy",

pkg/config/configobserver/secretconfigobserver.go

@@ -228,7 +228,7 @@ func (c *Controller) mergeConfig() {
 		cfg.Token = c.tokenConfig.Token
 	}
 
-	cfg.Report = len(cfg.Endpoint) > 0 && (len(cfg.Token) > 0 || len(cfg.Username) > 0)
+	cfg.Report = len(cfg.Endpoint) > 0 && len(cfg.Token) > 0
 	c.setConfig(&cfg)
 }
 

pkg/config/configobserver/secretconfigobserver_test.go

@@ -37,11 +37,9 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
 		expConfig *config.Controller
 		expErr    error
 	}{
-		{name: "interval too short",
+		{
+			name: "interval too short",
 			config: map[string]*corev1.Secret{
-				pullSecretKey: {Data: map[string][]byte{
-					".dockerconfigjson": nil,
-				}},
 				supportKey: {Data: map[string][]byte{
 					"username": []byte("someone"),
 					"password": []byte("secret"),
@@ -51,55 +49,45 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
 			},
 			expErr: fmt.Errorf("insights secret interval must be a duration (1h, 10m) greater than or equal to ten seconds: too short"),
 		},
-		{name: "interval incorrect format",
+		{
+			name: "interval incorrect format",
 			config: map[string]*corev1.Secret{
-				pullSecretKey: {Data: map[string][]byte{
-					".dockerconfigjson": nil,
-				}},
 				supportKey: {Data: map[string][]byte{
 					"interval": []byte("every second"),
 				}},
 			},
 			expErr: fmt.Errorf("insights secret interval must be a duration (1h, 10m) greater than or equal to ten seconds: time: invalid duration \"every second\""),
 		},
-		{name: "reportPullingDelay incorrect format",
+		{
+			name: "reportPullingDelay incorrect format",
 			config: map[string]*corev1.Secret{
-				pullSecretKey: {Data: map[string][]byte{
-					".dockerconfigjson": nil,
-				}},
 				supportKey: {Data: map[string][]byte{
 					"reportPullingDelay": []byte("every second"),
 				}},
 			},
 			expConfig: &config.Controller{}, // it only produces a warning in the log
 		},
-		{name: "reportMinRetryTime incorrect format",
+		{
+			name: "reportMinRetryTime incorrect format",
 			config: map[string]*corev1.Secret{
-				pullSecretKey: {Data: map[string][]byte{
-					".dockerconfigjson": nil,
-				}},
 				supportKey: {Data: map[string][]byte{
 					"reportMinRetryTime": []byte("every second"),
 				}},
 			},
 			expConfig: &config.Controller{}, // it only produces a warning in the log
 		},
-		{name: "reportPullingTimeout incorrect format",
+		{
+			name: "reportPullingTimeout incorrect format",
 			config: map[string]*corev1.Secret{
-				pullSecretKey: {Data: map[string][]byte{
-					".dockerconfigjson": nil,
-				}},
 				supportKey: {Data: map[string][]byte{
 					"reportPullingTimeout": []byte("every second"),
 				}},
 			},
 			expConfig: &config.Controller{}, // it only produces a warning in the log
 		},
-		{name: "correct interval",
+		{
+			name: "correct interval",
 			config: map[string]*corev1.Secret{
-				pullSecretKey: {Data: map[string][]byte{
-					".dockerconfigjson": nil,
-				}},
 				supportKey: {Data: map[string][]byte{
 					"interval": []byte("1m"),
 				}},
@@ -109,14 +97,14 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
 			},
 			expErr: nil,
 		},
-		{name: "set-all-config",
+		{
+			name: "set-all-config",
 			config: map[string]*corev1.Secret{
 				pullSecretKey: {Data: map[string][]byte{
-					".dockerconfigjson": nil,
+					".dockerconfigjson": []byte(`{"auths":{"cloud.openshift.com":{"auth":"testtoken","email":"test"}}}`),
 				}},
+
 				supportKey: {Data: map[string][]byte{
-					"username":                []byte("Dude"),
-					"password":                []byte("pswd123"),
 					"endpoint":                []byte("http://po.rt"),
 					"httpProxy":               []byte("http://pro.xy"),
 					"httpsProxy":              []byte("https://pro.xy"),
@@ -131,9 +119,8 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
 			},
 			expConfig: &config.Controller{
 				Report:   true,
-				Username: "Dude",
-				Password: "pswd123",
 				Endpoint: "http://po.rt",
+				Token:    "testtoken",
 				HTTPConfig: config.HTTPConfig{
 					HTTPProxy:  "http://pro.xy",
 					HTTPSProxy: "https://pro.xy",
@@ -211,7 +198,7 @@ func Test_ConfigObserver_ConfigChanged(t *testing.T) {
 			".dockerconfigjson": nil,
 		}},
 		supportKey: {Data: map[string][]byte{
-			"username": []byte("test2"),
+			"endpoint": []byte("test2"),
 		}},
 	})
 	// 1. config update
@@ -221,7 +208,7 @@ func Test_ConfigObserver_ConfigChanged(t *testing.T) {
 	}
 	// Check if the event arrived on the channel
 	if len(configCh) != 1 {
-		t.Fatalf("Config channel has more/less then 1 event on a singal config change. len(configCh)==%d", len(configCh))
+		t.Fatalf("Config channel has more/less than 1 event on a signal config change. len(configCh)==%d", len(configCh))
 	}
 
 	// Unsubscribe from config change
@@ -232,7 +219,7 @@ func Test_ConfigObserver_ConfigChanged(t *testing.T) {
 			".dockerconfigjson": nil,
 		}},
 		supportKey: {Data: map[string][]byte{
-			"username": []byte("test3"),
+			"endpoint": []byte("test3"),
 		}},
 	})
 	// 2. config update