Skip to content

Commit a6f01fb

Browse files
committed
CONSOLE-4292: Refactoring
1 parent 4039694 commit a6f01fb

File tree

5 files changed

+39
-102
lines changed

5 files changed

+39
-102
lines changed

Diff for: pkg/console/operator/operator.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ type consoleOperator struct {
8787
versionGetter status.VersionGetter
8888
// lister
8989
consolePluginLister listerv1.ConsolePluginLister
90+
// feature gate
9091

9192
resourceSyncer resourcesynccontroller.ResourceSyncer
9293

@@ -182,8 +183,7 @@ func NewConsoleOperator(
182183
versionGetter: versionGetter,
183184
// plugins
184185
consolePluginLister: consolePluginInformer.Lister(),
185-
186-
resourceSyncer: resourceSyncer,
186+
resourceSyncer: resourceSyncer,
187187

188188
monitoringDeploymentLister: monitoringDeploymentInformer.Lister(),
189189
}

Diff for: pkg/console/operatorclient/operatorclient.go

-86
This file was deleted.

Diff for: pkg/console/subresource/configmap/configmap.go

+8
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package configmap
33
import (
44
"fmt"
55
"net/url"
6+
"sort"
67

78
corev1 "k8s.io/api/core/v1"
89
"k8s.io/klog/v2"
@@ -146,13 +147,20 @@ func aggregateCSPDirectives(plugins []*v1.ConsolePlugin) map[v1.DirectiveType][]
146147
}
147148
}
148149
}
150+
// Check if the aggregated map is empty
151+
if len(aggregated) == 0 {
152+
return nil
153+
}
149154

150155
// Convert back to the desired format
151156
result := make(map[v1.DirectiveType][]string)
152157
for directive, valuesMap := range aggregated {
158+
result[directive] = make([]string, 0, len(valuesMap))
153159
for value := range valuesMap {
154160
result[directive] = append(result[directive], value)
155161
}
162+
// Sort the slice of strings for each directive
163+
sort.Strings(result[directive])
156164
}
157165

158166
return result

Diff for: pkg/console/subresource/configmap/configmap_test.go

+18-3
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package configmap
22

33
import (
44
"fmt"
5+
"sort"
56
"strconv"
67
"testing"
78

@@ -1343,9 +1344,6 @@ func TestAggregateCSPDirectives(t *testing.T) {
13431344
{
13441345
name: "Test aggregate CSP directives for a single ConsolePlugin",
13451346
input: []*consolev1.ConsolePlugin{
1346-
{
1347-
Spec: consolev1.ConsolePluginSpec{},
1348-
},
13491347
{
13501348
Spec: consolev1.ConsolePluginSpec{
13511349
ContentSecurityPolicy: []consolev1.ConsolePluginCSP{
@@ -1366,11 +1364,28 @@ func TestAggregateCSPDirectives(t *testing.T) {
13661364
consolev1.StyleSrc: {"style1", "style2"},
13671365
},
13681366
},
1367+
{
1368+
name: "Test a single ConsolePlugin without CSP directives",
1369+
input: []*consolev1.ConsolePlugin{
1370+
{
1371+
Spec: consolev1.ConsolePluginSpec{},
1372+
},
1373+
},
1374+
output: nil,
1375+
},
1376+
}
1377+
1378+
// Anonymous function to sort each slice in a directive map
1379+
sortDirectives := func(directives map[consolev1.DirectiveType][]string) {
1380+
for _, values := range directives {
1381+
sort.Strings(values)
1382+
}
13691383
}
13701384

13711385
for _, tt := range tests {
13721386
t.Run(tt.name, func(t *testing.T) {
13731387
result := aggregateCSPDirectives(tt.input)
1388+
sortDirectives(result)
13741389
if diff := deep.Equal(tt.output, result); diff != nil {
13751390
t.Error(diff)
13761391
t.Errorf("Got: %v \n", result)

Diff for: test/e2e/plugins_test.go

+11-11
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
consoleapi "github.com/openshift/console-operator/pkg/api"
1111
"github.com/openshift/console-operator/pkg/console/subresource/consoleserver"
1212
"github.com/openshift/console-operator/test/e2e/framework"
13-
"golang.org/x/exp/maps"
1413
yaml "gopkg.in/yaml.v2"
1514
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1615
"k8s.io/apimachinery/pkg/util/wait"
@@ -61,16 +60,17 @@ func cleanupTestCase(t *testing.T, client *framework.ClientSet, defaultPlugins,
6160
}
6261

6362
// TestPluginsCSPAggregation verifies correct CSP aggregation from multiple plugins.
64-
func TestPluginsCSPAggregation(t *testing.T) {
65-
client, defaultPlugins := setupTestCase(t)
66-
defer cleanupTestCase(t, client, defaultPlugins, maps.Keys(pluginCSPs))
67-
68-
for name, csp := range pluginCSPs {
69-
createConsolePlugin(t, client, getPlugin(name, csp))
70-
}
71-
setOperatorConfigPlugins(t, client, maps.Keys(pluginCSPs))
72-
verifyConsoleConfigCSP(t, client, expectedCSP)
73-
}
63+
// Uncomment this test once the ConsoleContentSecurityPolicy is not behind feature gate.
64+
// func TestPluginsCSPAggregation(t *testing.T) {
65+
// client, defaultPlugins := setupTestCase(t)
66+
// defer cleanupTestCase(t, client, defaultPlugins, maps.Keys(pluginCSPs))
67+
68+
// for name, csp := range pluginCSPs {
69+
// createConsolePlugin(t, client, getPlugin(name, csp))
70+
// }
71+
// setOperatorConfigPlugins(t, client, maps.Keys(pluginCSPs))
72+
// verifyConsoleConfigCSP(t, client, expectedCSP)
73+
// }
7474

7575
// TestAddPlugins tests addition of available and unavailable plugins.
7676
func TestAddPlugins(t *testing.T) {

0 commit comments

Comments
 (0)