Skip to content

Commit e0f175b

Browse files
openshift-cherrypick-robottremes
openshift-cherrypick-robot
and
tremes
authored
[release-4.14] OCPBUGS-21859: remove username & password config options (#843)
* OCPBUGS-21797: remove username & password config options * review update --------- Co-authored-by: Tomáš Remeš <[email protected]>
1 parent e967e1d commit e0f175b

File tree

7 files changed

+22
-97
lines changed

7 files changed

+22
-97
lines changed

Diff for: ‎docs/arch.md

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ The `support` secret provides following configuration attributes:
1010
- `endpoint` - upload endpoint - default is `https://console.redhat.com/api/ingress/v1/upload`,
1111
- `interval` - data gathering & uploading frequency - default is `2h`
1212
- `httpProxy`, `httpsProxy`, `noProxy` eventually to set custom proxy, which overrides cluster proxy just for the Insights Operator
13-
- `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.
1413
- `enableGlobalObfuscation` - to enable the global obfuscation of the IP addresses and the cluster domain name. Default value is `false`
1514
- `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)
1615
- `reportPullingDelay` - the delay between data upload and download. Default value is `60s`

Diff for: ‎pkg/authorizer/clusterauthorizer/clusterauthorizer.go

-7
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,10 @@ func New(configurator configobserver.Configurator) *Authorizer {
2828

2929
// Authorize adds the necessary auth header to the request, depending on the config. (BasicAuth/Token)
3030
func (a *Authorizer) Authorize(req *http.Request) error {
31-
cfg := a.configurator.Config()
32-
3331
if req.Header == nil {
3432
req.Header = make(http.Header)
3533
}
3634

37-
if len(cfg.Username) > 0 || len(cfg.Password) > 0 {
38-
req.SetBasicAuth(cfg.Username, cfg.Password)
39-
return nil
40-
}
41-
4235
token, err := a.Token()
4336
if err != nil {
4437
return err

Diff for: ‎pkg/config/config.go

+1-11
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,7 @@ type Controller struct {
5353
// To see the detailed info about how anonymization works, go to the docs of package anonymization.
5454
EnableGlobalObfuscation bool
5555

56-
Username string
57-
Password string
58-
Token string
56+
Token string
5957

6058
HTTPConfig HTTPConfig
6159
OCMConfig OCMConfig
@@ -89,7 +87,6 @@ func (c *Controller) ToString() string {
8987
"endpoint=%s "+
9088
"conditional_gatherer_endpoint=%s "+
9189
"interval=%s "+
92-
"username=%t "+
9390
"token=%t "+
9491
"reportEndpoint=%s "+
9592
"initialPollingDelay=%s "+
@@ -100,7 +97,6 @@ func (c *Controller) ToString() string {
10097
c.Endpoint,
10198
c.ConditionalGathererEndpoint,
10299
c.Interval,
103-
len(c.Username) > 0,
104100
len(c.Token) > 0,
105101
c.ReportEndpoint,
106102
c.ReportPullingDelay,
@@ -110,7 +106,6 @@ func (c *Controller) ToString() string {
110106
}
111107

112108
func (c *Controller) MergeWith(cfg *Controller) {
113-
c.mergeCredentials(cfg)
114109
c.mergeInterval(cfg)
115110
c.mergeEndpoint(cfg)
116111
c.mergeConditionalGathererEndpoint(cfg)
@@ -121,11 +116,6 @@ func (c *Controller) MergeWith(cfg *Controller) {
121116
c.mergeReportEndpointTechPreview(cfg)
122117
}
123118

124-
func (c *Controller) mergeCredentials(cfg *Controller) {
125-
c.Username = cfg.Username
126-
c.Password = cfg.Password
127-
}
128-
129119
func (c *Controller) mergeEndpoint(cfg *Controller) {
130120
if len(cfg.Endpoint) > 0 {
131121
c.Endpoint = cfg.Endpoint

Diff for: ‎pkg/config/configobserver/config.go

-10
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ func LoadConfigFromSecret(secret *v1.Secret) (config.Controller, error) {
2424
var cfg Config
2525
var err error
2626

27-
cfg.loadCredentials(secret.Data)
2827
cfg.loadEndpoint(secret.Data)
2928
cfg.loadConditionalGathererEndpoint(secret.Data)
3029
cfg.loadHTTP(secret.Data)
@@ -52,15 +51,6 @@ func LoadConfigFromSecret(secret *v1.Secret) (config.Controller, error) {
5251
return cfg.Controller, err
5352
}
5453

55-
func (c *Config) loadCredentials(data map[string][]byte) {
56-
if username, ok := data["username"]; ok {
57-
c.Username = strings.TrimSpace(string(username))
58-
}
59-
if password, ok := data["password"]; ok {
60-
c.Password = strings.TrimSpace(string(password))
61-
}
62-
}
63-
6454
func (c *Config) loadEndpoint(data map[string][]byte) {
6555
if endpoint, ok := data["endpoint"]; ok {
6656
c.Endpoint = string(endpoint)

Diff for: ‎pkg/config/configobserver/config_test.go

-34
Original file line numberDiff line numberDiff line change
@@ -10,36 +10,6 @@ import (
1010
"github.com/openshift/insights-operator/pkg/config"
1111
)
1212

13-
func TestConfig_loadCredentials(t *testing.T) {
14-
tests := []struct {
15-
name string
16-
data map[string][]byte
17-
want *Config
18-
}{
19-
{
20-
name: "Load credentials",
21-
data: map[string][]byte{
22-
"username": []byte("user"),
23-
"password": []byte("xxxxxx"),
24-
},
25-
want: &Config{Controller: config.Controller{
26-
Report: false,
27-
Username: "user",
28-
Password: "xxxxxx",
29-
}},
30-
},
31-
}
32-
for _, tt := range tests {
33-
t.Run(tt.name, func(t *testing.T) {
34-
got := &Config{Controller: config.Controller{}}
35-
got.loadCredentials(tt.data)
36-
if !reflect.DeepEqual(got, tt.want) {
37-
t.Errorf("loadCredentials() got = %v, want %v", got, tt.want)
38-
}
39-
})
40-
}
41-
}
42-
4313
func TestConfig_loadEndpoint(t *testing.T) {
4414
tests := []struct {
4515
name string
@@ -194,8 +164,6 @@ func TestLoadConfigFromSecret(t *testing.T) {
194164
name: "Can load from secret",
195165
secret: &v1.Secret{
196166
Data: map[string][]byte{
197-
"username": []byte("user"),
198-
"password": []byte("xxxxxx"),
199167
"endpoint": []byte("http://endpoint"),
200168
"noProxy": []byte("no-proxy"),
201169
"reportEndpoint": []byte("http://report"),
@@ -206,8 +174,6 @@ func TestLoadConfigFromSecret(t *testing.T) {
206174
Report: true,
207175
Endpoint: "http://endpoint",
208176
ReportEndpoint: "http://report",
209-
Username: "user",
210-
Password: "xxxxxx",
211177
ReportPullingDelay: time.Duration(-1),
212178
HTTPConfig: config.HTTPConfig{
213179
NoProxy: "no-proxy",

Diff for: ‎pkg/config/configobserver/secretconfigobserver.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func (c *Controller) mergeConfig() {
228228
cfg.Token = c.tokenConfig.Token
229229
}
230230

231-
cfg.Report = len(cfg.Endpoint) > 0 && (len(cfg.Token) > 0 || len(cfg.Username) > 0)
231+
cfg.Report = len(cfg.Endpoint) > 0 && len(cfg.Token) > 0
232232
c.setConfig(&cfg)
233233
}
234234

Diff for: ‎pkg/config/configobserver/secretconfigobserver_test.go

+20-33
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,9 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
3737
expConfig *config.Controller
3838
expErr error
3939
}{
40-
{name: "interval too short",
40+
{
41+
name: "interval too short",
4142
config: map[string]*corev1.Secret{
42-
pullSecretKey: {Data: map[string][]byte{
43-
".dockerconfigjson": nil,
44-
}},
4543
supportKey: {Data: map[string][]byte{
4644
"username": []byte("someone"),
4745
"password": []byte("secret"),
@@ -51,55 +49,45 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
5149
},
5250
expErr: fmt.Errorf("insights secret interval must be a duration (1h, 10m) greater than or equal to ten seconds: too short"),
5351
},
54-
{name: "interval incorrect format",
52+
{
53+
name: "interval incorrect format",
5554
config: map[string]*corev1.Secret{
56-
pullSecretKey: {Data: map[string][]byte{
57-
".dockerconfigjson": nil,
58-
}},
5955
supportKey: {Data: map[string][]byte{
6056
"interval": []byte("every second"),
6157
}},
6258
},
6359
expErr: fmt.Errorf("insights secret interval must be a duration (1h, 10m) greater than or equal to ten seconds: time: invalid duration \"every second\""),
6460
},
65-
{name: "reportPullingDelay incorrect format",
61+
{
62+
name: "reportPullingDelay incorrect format",
6663
config: map[string]*corev1.Secret{
67-
pullSecretKey: {Data: map[string][]byte{
68-
".dockerconfigjson": nil,
69-
}},
7064
supportKey: {Data: map[string][]byte{
7165
"reportPullingDelay": []byte("every second"),
7266
}},
7367
},
7468
expConfig: &config.Controller{}, // it only produces a warning in the log
7569
},
76-
{name: "reportMinRetryTime incorrect format",
70+
{
71+
name: "reportMinRetryTime incorrect format",
7772
config: map[string]*corev1.Secret{
78-
pullSecretKey: {Data: map[string][]byte{
79-
".dockerconfigjson": nil,
80-
}},
8173
supportKey: {Data: map[string][]byte{
8274
"reportMinRetryTime": []byte("every second"),
8375
}},
8476
},
8577
expConfig: &config.Controller{}, // it only produces a warning in the log
8678
},
87-
{name: "reportPullingTimeout incorrect format",
79+
{
80+
name: "reportPullingTimeout incorrect format",
8881
config: map[string]*corev1.Secret{
89-
pullSecretKey: {Data: map[string][]byte{
90-
".dockerconfigjson": nil,
91-
}},
9282
supportKey: {Data: map[string][]byte{
9383
"reportPullingTimeout": []byte("every second"),
9484
}},
9585
},
9686
expConfig: &config.Controller{}, // it only produces a warning in the log
9787
},
98-
{name: "correct interval",
88+
{
89+
name: "correct interval",
9990
config: map[string]*corev1.Secret{
100-
pullSecretKey: {Data: map[string][]byte{
101-
".dockerconfigjson": nil,
102-
}},
10391
supportKey: {Data: map[string][]byte{
10492
"interval": []byte("1m"),
10593
}},
@@ -109,14 +97,14 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
10997
},
11098
expErr: nil,
11199
},
112-
{name: "set-all-config",
100+
{
101+
name: "set-all-config",
113102
config: map[string]*corev1.Secret{
114103
pullSecretKey: {Data: map[string][]byte{
115-
".dockerconfigjson": nil,
104+
".dockerconfigjson": []byte(`{"auths":{"cloud.openshift.com":{"auth":"testtoken","email":"test"}}}`),
116105
}},
106+
117107
supportKey: {Data: map[string][]byte{
118-
"username": []byte("Dude"),
119-
"password": []byte("pswd123"),
120108
"endpoint": []byte("http://po.rt"),
121109
"httpProxy": []byte("http://pro.xy"),
122110
"httpsProxy": []byte("https://pro.xy"),
@@ -131,9 +119,8 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
131119
},
132120
expConfig: &config.Controller{
133121
Report: true,
134-
Username: "Dude",
135-
Password: "pswd123",
136122
Endpoint: "http://po.rt",
123+
Token: "testtoken",
137124
HTTPConfig: config.HTTPConfig{
138125
HTTPProxy: "http://pro.xy",
139126
HTTPSProxy: "https://pro.xy",
@@ -211,7 +198,7 @@ func Test_ConfigObserver_ConfigChanged(t *testing.T) {
211198
".dockerconfigjson": nil,
212199
}},
213200
supportKey: {Data: map[string][]byte{
214-
"username": []byte("test2"),
201+
"endpoint": []byte("test2"),
215202
}},
216203
})
217204
// 1. config update
@@ -221,7 +208,7 @@ func Test_ConfigObserver_ConfigChanged(t *testing.T) {
221208
}
222209
// Check if the event arrived at the channel
223210
if len(configCh) != 1 {
224-
t.Fatalf("Config channel has more/less then 1 event on a singal config change. len(configCh)==%d", len(configCh))
211+
t.Fatalf("Config channel has more/less than 1 event on a signal config change. len(configCh)==%d", len(configCh))
225212
}
226213

227214
// Unsubscribe from config change
@@ -232,7 +219,7 @@ func Test_ConfigObserver_ConfigChanged(t *testing.T) {
232219
".dockerconfigjson": nil,
233220
}},
234221
supportKey: {Data: map[string][]byte{
235-
"username": []byte("test3"),
222+
"endpoint": []byte("test3"),
236223
}},
237224
})
238225
// 2. config update

0 commit comments

Comments
 (0)