CONSOLE 2919: code review adjustments for managed cluster action/views

bindata/configmaps/console-managed-cluster-ingress-cert-configmap.yaml

@@ -1,4 +1,4 @@
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  namespace: openshift-config-managed
+  namespace: openshift-console

pkg/console/operator/sync_v400.go

@@ -71,17 +71,17 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact
 		return statusHandler.FlushAndReturn(routeErr)
 	}
 
-	cm, cmChanged, cmErrReason, cmErr := co.SyncConfigMap(ctx, set.Operator, set.Console, set.Infrastructure, set.OAuth, route, controllerContext.Recorder())
+	// managed-clusters ConfigMap is managed by another controller and is not required, we don't need to exit the sync loop if it's not present
+	canMountManagedClusterConfig, managedClusterConfigErrReason, managedClusterConfigErr := co.SyncManagedClusterConfigMap(ctx)
+	statusHandler.AddConditions(status.HandleProgressingOrDegraded("ManagedClusterConfigSync", managedClusterConfigErrReason, managedClusterConfigErr))
+
+	cm, cmChanged, cmErrReason, cmErr := co.SyncConfigMap(ctx, set.Operator, set.Console, set.Infrastructure, set.OAuth, route, canMountManagedClusterConfig, controllerContext.Recorder())
 	toUpdate = toUpdate || cmChanged
 	statusHandler.AddConditions(status.HandleProgressingOrDegraded("ConfigMapSync", cmErrReason, cmErr))
 	if cmErr != nil {
 		return statusHandler.FlushAndReturn(cmErr)
 	}
 
-	// managed-clusters ConfigMap is managed by another controller and is not required, we don't need to exit the sync loop if it's not present
-	canMountManagedClusterConfig, managedClusterConfigErrReason, managedClusterConfigErr := co.SyncManagedClusterConfigMap(ctx)
-	statusHandler.AddConditions(status.HandleProgressingOrDegraded("ManagedClusterConfigSync", managedClusterConfigErrReason, managedClusterConfigErr))
-
 	serviceCAConfigMap, serviceCAChanged, serviceCAErrReason, serviceCAErr := co.SyncServiceCAConfigMap(ctx, set.Operator)
 	toUpdate = toUpdate || serviceCAChanged
 	statusHandler.AddConditions(status.HandleProgressingOrDegraded("ServiceCASync", serviceCAErrReason, serviceCAErr))
@@ -317,6 +317,7 @@ func (co *consoleOperator) SyncConfigMap(
 	infrastructureConfig *configv1.Infrastructure,
 	oauthConfig *configv1.OAuth,
 	activeConsoleRoute *routev1.Route,
+	canMountManagedClusterConfig bool,
 	recorder events.Recorder,
 ) (consoleConfigMap *corev1.ConfigMap, changed bool, reason string, err error) {
 
@@ -349,7 +350,7 @@ func (co *consoleOperator) SyncConfigMap(
 	}
 
 	pluginsEndpointMap := co.GetPluginsEndpointMap(operatorConfig.Spec.Plugins)
-	defaultConfigmap, _, err := configmapsub.DefaultConfigMap(operatorConfig, consoleConfig, managedConfig, infrastructureConfig, activeConsoleRoute, useDefaultCAFile, inactivityTimeoutSeconds, pluginsEndpointMap)
+	defaultConfigmap, _, err := configmapsub.DefaultConfigMap(operatorConfig, consoleConfig, managedConfig, infrastructureConfig, activeConsoleRoute, useDefaultCAFile, inactivityTimeoutSeconds, pluginsEndpointMap, canMountManagedClusterConfig)
 	if err != nil {
 		return nil, false, "FailedConsoleConfigBuilder", err
 	}
@@ -380,7 +381,7 @@ func (co *consoleOperator) SyncManagedClusterConfigMap(ctx context.Context) (boo
 	// Degraded if managed cluster config map is present but doesn't have the correct data key
 	_, ok := managedClusterConfigMap.Data[api.ManagedClusterConfigKey]
 	if !ok {
-		return false, "MissingManagedClusterConfig", fmt.Errorf("%v ConfigMap is missing %v data key", api.ManagedClusterConfigMapName, api.ManagedClusterConfigKey)
+		return false, "MissingManagedClusterConfig", fmt.Errorf("%s ConfigMap is missing %s data key", api.ManagedClusterConfigMapName, api.ManagedClusterConfigKey)
 	}
 
 	// Managed cluster config map is present and can be mounted

pkg/console/subresource/configmap/configmap.go

@@ -43,7 +43,13 @@ func DefaultConfigMap(
 	activeConsoleRoute *routev1.Route,
 	useDefaultCAFile bool,
 	inactivityTimeoutSeconds int,
-	pluginsEndpoingMap map[string]string) (consoleConfigmap *corev1.ConfigMap, unsupportedOverridesHaveMerged bool, err error) {
+	pluginsEndpoingMap map[string]string,
+	canMountManagedClusterConfig bool) (consoleConfigmap *corev1.ConfigMap, unsupportedOverridesHaveMerged bool, err error) {
+
+	managedClusterConfigFile := ""
+	if canMountManagedClusterConfig {
+		managedClusterConfigFile = fmt.Sprintf("%s/%s", api.ManagedClusterConfigMountDir, api.ManagedClusterConfigKey)
+	}
 
 	defaultBuilder := &consoleserver.ConsoleServerCLIConfigBuilder{}
 	defaultConfig, err := defaultBuilder.Host(activeConsoleRoute.Spec.Host).
@@ -53,7 +59,7 @@ func DefaultConfigMap(
 		OAuthServingCert(useDefaultCAFile).
 		APIServerURL(getApiUrl(infrastructureConfig)).
 		InactivityTimeout(inactivityTimeoutSeconds).
-		ManagedClusterConfigFile(fmt.Sprintf("%v/%v", api.ManagedClusterConfigMountDir, api.ManagedClusterConfigKey)).
+		ManagedClusterConfigFile(managedClusterConfigFile).
 		ConfigYAML()
 	if err != nil {
 		klog.Errorf("failed to generate default console-config config: %v", err)

pkg/console/subresource/configmap/configmap_test.go

@@ -30,14 +30,15 @@ const (
 // To manually run these tests: go test -v ./pkg/console/subresource/configmap/...
 func TestDefaultConfigMap(t *testing.T) {
 	type args struct {
-		operatorConfig           *operatorv1.Console
-		consoleConfig            *configv1.Console
-		managedConfig            *corev1.ConfigMap
-		infrastructureConfig     *configv1.Infrastructure
-		rt                       *routev1.Route
-		useDefaultCAFile         bool
-		inactivityTimeoutSeconds int
-		enabledPlugins           map[string]string
+		operatorConfig               *operatorv1.Console
+		consoleConfig                *configv1.Console
+		managedConfig                *corev1.ConfigMap
+		infrastructureConfig         *configv1.Infrastructure
+		rt                           *routev1.Route
+		useDefaultCAFile             bool
+		inactivityTimeoutSeconds     int
+		enabledPlugins               map[string]string
+		canMountManagedClusterConfig bool
 	}
 	tests := []struct {
 		name string
@@ -63,8 +64,9 @@ func TestDefaultConfigMap(t *testing.T) {
 						Host: host,
 					},
 				},
-				useDefaultCAFile:         true,
-				inactivityTimeoutSeconds: 0,
+				useDefaultCAFile:             true,
+				inactivityTimeoutSeconds:     0,
+				canMountManagedClusterConfig: true,
 			},
 			want: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
@@ -114,8 +116,9 @@ providers: {}
 						Host: host,
 					},
 				},
-				useDefaultCAFile:         false,
-				inactivityTimeoutSeconds: 0,
+				useDefaultCAFile:             false,
+				inactivityTimeoutSeconds:     0,
+				canMountManagedClusterConfig: true,
 			},
 			want: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
@@ -174,8 +177,9 @@ customization:
 						Host: host,
 					},
 				},
-				useDefaultCAFile:         true,
-				inactivityTimeoutSeconds: 0,
+				useDefaultCAFile:             true,
+				inactivityTimeoutSeconds:     0,
+				canMountManagedClusterConfig: true,
 			},
 			want: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
@@ -243,8 +247,9 @@ customization:
 						Host: host,
 					},
 				},
-				useDefaultCAFile:         true,
-				inactivityTimeoutSeconds: 0,
+				useDefaultCAFile:             true,
+				inactivityTimeoutSeconds:     0,
+				canMountManagedClusterConfig: true,
 			},
 			want: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
@@ -317,8 +322,9 @@ customization:
 						Host: host,
 					},
 				},
-				useDefaultCAFile:         true,
-				inactivityTimeoutSeconds: 0,
+				useDefaultCAFile:             true,
+				inactivityTimeoutSeconds:     0,
+				canMountManagedClusterConfig: true,
 			},
 			want: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
@@ -393,8 +399,9 @@ customization:
 						Host: host,
 					},
 				},
-				useDefaultCAFile:         true,
-				inactivityTimeoutSeconds: 0,
+				useDefaultCAFile:             true,
+				inactivityTimeoutSeconds:     0,
+				canMountManagedClusterConfig: true,
 			},
 			want: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
@@ -451,8 +458,9 @@ providers:
 						Host: customHostname,
 					},
 				},
-				useDefaultCAFile:         false,
-				inactivityTimeoutSeconds: 0,
+				useDefaultCAFile:             false,
+				inactivityTimeoutSeconds:     0,
+				canMountManagedClusterConfig: true,
 			},
 			want: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
@@ -503,8 +511,9 @@ providers: {}
 						Host: host,
 					},
 				},
-				useDefaultCAFile:         true,
-				inactivityTimeoutSeconds: 60,
+				useDefaultCAFile:             true,
+				inactivityTimeoutSeconds:     60,
+				canMountManagedClusterConfig: true,
 			},
 			want: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
@@ -561,6 +570,7 @@ providers: {}
 					"plugin1": "plugin1_url",
 					"plugin2": "plugin2_url",
 				},
+				canMountManagedClusterConfig: true,
 			},
 			want: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
@@ -590,6 +600,57 @@ providers: {}
 plugins:
   plugin1: plugin1_url
   plugin2: plugin2_url
+`,
+				},
+			},
+		},
+		{
+			name: "Test canMountManagedClusterConfig set to false",
+			args: args{
+				operatorConfig: &operatorv1.Console{},
+				consoleConfig:  &configv1.Console{},
+				managedConfig:  &corev1.ConfigMap{},
+				infrastructureConfig: &configv1.Infrastructure{
+					Status: configv1.InfrastructureStatus{
+						APIServerURL: mockAPIServer,
+					},
+				},
+				rt: &routev1.Route{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: api.OpenShiftConsoleName,
+					},
+					Spec: routev1.RouteSpec{
+						Host: host,
+					},
+				},
+				useDefaultCAFile:             true,
+				inactivityTimeoutSeconds:     0,
+				canMountManagedClusterConfig: false,
+			},
+			want: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        api.OpenShiftConsoleConfigMapName,
+					Namespace:   api.OpenShiftConsoleNamespace,
+					Labels:      map[string]string{"app": api.OpenShiftConsoleName},
+					Annotations: map[string]string{},
+				},
+				Data: map[string]string{configKey: `kind: ConsoleConfig
+apiVersion: console.openshift.io/v1
+auth:
+  clientID: console
+  clientSecretFile: /var/oauth-config/clientSecret
+  oauthEndpointCAFile: /var/run/secrets/kubernetes.io/serviceaccount/ca.crt
+clusterInfo:
+  consoleBaseAddress: https://` + host + `
+  masterPublicURL: ` + mockAPIServer + `
+customization:
+  branding: ` + DEFAULT_BRAND + `
+  documentationBaseURL: ` + DEFAULT_DOC_URL + `
+servingInfo:
+  bindAddress: https://[::]:8443
+  certFile: /var/serving-cert/tls.crt
+  keyFile: /var/serving-cert/tls.key
+providers: {}
 `,
 				},
 			},
@@ -606,6 +667,7 @@ plugins:
 				tt.args.useDefaultCAFile,
 				tt.args.inactivityTimeoutSeconds,
 				tt.args.enabledPlugins,
+				tt.args.canMountManagedClusterConfig,
 			)
 
 			// marshall the exampleYaml to map[string]interface{} so we can use it in diff below

pkg/console/subresource/configmap/managed_clusters_test.go

@@ -20,7 +20,7 @@ var (
 			URL:    "test-url",
 			CAFile: "/var/api/ca",
 		},
-		Oauth: consoleserver.ManagedClusterOAuthConfig{
+		OAuth: consoleserver.ManagedClusterOAuthConfig{
 			ClientID:     "test-client-id",
 			ClientSecret: "test-client-secret",
 			CAFile:       "/var/oauth/ca",

pkg/console/subresource/consoleserver/types.go

@@ -143,5 +143,5 @@ type ManagedClusterOAuthConfig struct {
 type ManagedClusterConfig struct {
 	Name      string                        `json:"name" yaml:"name"` // ManagedCluster name, provided through ACM
 	APIServer ManagedClusterAPIServerConfig `json:"apiServer" yaml:"apiServer"`
-	Oauth     ManagedClusterOAuthConfig     `json:"oauth" yaml:"oauth"`
+	OAuth     ManagedClusterOAuthConfig     `json:"oauth" yaml:"oauth"`
 }

pkg/console/subresource/managedclusterview/default-ingress-cert.go → pkg/console/subresource/managedcluster/default_ingress_cert.go

@@ -1,4 +1,4 @@
-package managedclusterview
+package managedcluster
 
 import (
 	operatorv1 "github.com/openshift/api/operator/v1"

pkg/console/subresource/managedclusterview/oauth-client.go → pkg/console/subresource/managedcluster/oauth_client.go

@@ -1,18 +1,33 @@
-package managedclusterview
+package managedcluster
 
 import (
 	operatorv1 "github.com/openshift/api/operator/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	// openshift
-
 	"github.com/openshift/console-operator/pkg/console/assets"
 	"github.com/openshift/console-operator/pkg/console/subresource/util"
 	// acm - TODO conflicts adding package to go.mod with several dependencies
 	// managedclusterviewv1beta1 "github.com/open-cluster-management/multicloud-operators-foundation/pkg/apis/action/v1beta1"
 )
 
+func DefaultCreateOAuthClient(cr *operatorv1.Console, cn string, sec string, redirects []string) *unstructured.Unstructured {
+	managedClusterAction := CreateOAuthClientStub(cn)
+	withDefaultCreateOAuthClientInfo(managedClusterAction, cn, sec, redirects)
+	return managedClusterAction
+}
+
+func withDefaultCreateOAuthClientInfo(mca *unstructured.Unstructured, cn string, sec string, redirects []string) {
+	unstructured.SetNestedField(mca.Object, cn, "metadata", "namespace")
+	unstructured.SetNestedField(mca.Object, sec, "spec", "kube", "template", "secret")
+	unstructured.SetNestedStringSlice(mca.Object, redirects, "spec", "kube", "template", "redirectURIs")
+}
+
+func CreateOAuthClientStub(cn string) *unstructured.Unstructured {
+	return util.ReadUnstructuredOrDie(assets.MustAsset("managedclusteractions/console-managed-cluster-action-create-oauth-client.yaml"))
+}
+
 func DefaultViewOAuthClient(cr *operatorv1.Console, cn string) *unstructured.Unstructured {
 	managedClusterView := ViewOAuthClientStub(cn)
 	withDefaultViewOAuthClientInfo(managedClusterView, cn)
@@ -27,6 +42,22 @@ func ViewOAuthClientStub(cn string) *unstructured.Unstructured {
 	return util.ReadUnstructuredOrDie(assets.MustAsset("managedclusterviews/console-managed-cluster-view-oauth-client.yaml"))
 }
 
+func GetActionGroupVersionResource() schema.GroupVersionResource {
+	return schema.GroupVersionResource{
+		Group:    "view.open-cluster-management.io",
+		Version:  "v1beta1",
+		Resource: "managedclusterviews",
+	}
+}
+
+func GetViewGroupVersionResource() schema.GroupVersionResource {
+	return schema.GroupVersionResource{
+		Group:    "action.open-cluster-management.io",
+		Version:  "v1beta1",
+		Resource: "managedclusteractions",
+	}
+}
+
 func GetStatus(mcv *unstructured.Unstructured) (bool, error) {
 	conditions, found, err := unstructured.NestedSlice(mcv.Object, "status", "conditions")
 	if err != nil || !found || len(conditions) == 0 {
@@ -57,11 +88,3 @@ func GetNamespace(mcv *unstructured.Unstructured) (string, error) {
 	}
 	return namespace, nil
 }
-
-func GetGroupVersionResource() schema.GroupVersionResource {
-	return schema.GroupVersionResource{
-		Group:    "view.open-cluster-management.io",
-		Version:  "v1beta1",
-		Resource: "managedclusterviews",
-	}
-}