Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 54 additions & 6 deletions cmd/ocm-backplane/console/console.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,16 @@ import (
"github.com/Masterminds/semver"
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
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"
"github.com/pkg/browser"
logger "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/cli-runtime/pkg/genericclioptions"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"

Expand All @@ -51,6 +53,12 @@ import (
"github.com/openshift/backplane-cli/pkg/utils"
)

var consolePluginV1Alpha1Resource = schema.GroupVersionResource{
Group: "console.openshift.io",
Version: "v1alpha1",
Resource: "consoleplugins",
}

type execActionOnTermInterface interface {
execActionOnTerminationFunction(action postTerminateFunc) error
}
Expand Down Expand Up @@ -797,32 +805,72 @@ func getConsolePluginFromCluster(config *rest.Config) ([]string, error) {

// getConsolePluginFrom411Cluster get the consoleplugin from the cluster with version lt 4.12
func getConsolePluginFrom411Cluster(config *rest.Config) ([]string, error) {
consoleInterface, err := consolev1alpha1typedclient.NewForConfig(config)
consoleInterface, err := dynamic.NewForConfig(config)

if err != nil {
return nil, err
}
consolePlugins, err := consoleInterface.ConsolePlugins().List(context.TODO(), metav1.ListOptions{})
consolePlugins, err := consoleInterface.Resource(consolePluginV1Alpha1Resource).List(context.TODO(), metav1.ListOptions{})
if err != nil {
return nil, err
}

var enabledPlugins []string

for _, cp := range consolePlugins.Items {
enabled, err := isConsolePluginEnabled(config, cp.Name)
enabled, err := isConsolePluginEnabled(config, cp.GetName())
if err != nil {
return nil, err
}
if enabled {
enabledPlugins = append(enabledPlugins, fmt.Sprintf("%s=https://%s.%s.svc.cluster.local:%d%s",
cp.Name, cp.Spec.Service.Name, cp.Spec.Service.Namespace, cp.Spec.Service.Port, cp.Spec.Service.BasePath))
pluginURL, err := consolePluginV1Alpha1URL(cp)
if err != nil {
return nil, err
}
enabledPlugins = append(enabledPlugins, pluginURL)
}
}

return enabledPlugins, nil
}

func consolePluginV1Alpha1URL(consolePlugin unstructured.Unstructured) (string, error) {
serviceName, found, err := unstructured.NestedString(consolePlugin.Object, "spec", "service", "name")
if err != nil {
return "", fmt.Errorf("failed to read spec.service.name from console plugin %q: %w", consolePlugin.GetName(), err)
}
if !found {
return "", fmt.Errorf("console plugin %q is missing spec.service.name", consolePlugin.GetName())
}

serviceNamespace, found, err := unstructured.NestedString(consolePlugin.Object, "spec", "service", "namespace")
if err != nil {
return "", fmt.Errorf("failed to read spec.service.namespace from console plugin %q: %w", consolePlugin.GetName(), err)
}
if !found {
return "", fmt.Errorf("console plugin %q is missing spec.service.namespace", consolePlugin.GetName())
}

servicePort, found, err := unstructured.NestedInt64(consolePlugin.Object, "spec", "service", "port")
if err != nil {
return "", fmt.Errorf("failed to read spec.service.port from console plugin %q: %w", consolePlugin.GetName(), err)
}
if !found {
return "", fmt.Errorf("console plugin %q is missing spec.service.port", consolePlugin.GetName())
}

basePath, found, err := unstructured.NestedString(consolePlugin.Object, "spec", "service", "basePath")
if err != nil {
return "", fmt.Errorf("failed to read spec.service.basePath from console plugin %q: %w", consolePlugin.GetName(), err)
}
if !found {
return "", fmt.Errorf("console plugin %q is missing spec.service.basePath", consolePlugin.GetName())
}

return fmt.Sprintf("%s=https://%s.%s.svc.cluster.local:%d%s",
consolePlugin.GetName(), serviceName, serviceNamespace, servicePort, basePath), nil
}

Comment on lines +837 to +873

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redhat-chai-bot this is a large implementation for a resource which is simple originally. Is there a way to simplify it? Is implementing it in dynamic a must? instead of using the original client-go one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for raising this. I checked the exact client revisions in this PR:

  • The older github.com/openshift/client-go revision contains the typed console/.../v1alpha1 client, but the compatible revision selected for the ocm-cli v1.0.17 dependency graph no longer contains that package; it retains the typed ConsolePlugin client under console/.../v1.
  • ocm-cli v1.0.17 requires k8s.io/apimachinery v0.34.3. The older OpenShift client-go module is tied to Kubernetes 0.25 and structured-merge-diff/v4, while the compatible graph uses Kubernetes 0.34 and structured-merge-diff/v6. Restoring the older typed client would reintroduce the incompatibility that caused the original dependency-update build to fail.
  • The legacy cluster resource is still console.openshift.io/v1alpha1, so this path uses the dynamic client with an explicit GVR and a small helper to read the four service fields. The tests cover the GVR, JSON decoding, enabled/disabled filtering, API errors, and invalid fields.

For this dependency set, the dynamic client is necessary; reverting to the old typed client is not viable without reverting the compatible client dependency graph. The helper can still be simplified structurally if there is a preferred local pattern, but the client choice is driven by the API/module change.


AI-generated. Review for accuracy.

// isConsolePluginEnabled checks if the consoleplugin object is enabled in console.operator
func isConsolePluginEnabled(config *rest.Config, consolePlugin string) (bool, error) {
operatorInterface, err := operatorv1typedclient.NewForConfig(config)
Expand Down
219 changes: 218 additions & 1 deletion cmd/ocm-backplane/console/console_test.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
package console

import (
"errors"
"fmt"
"net/http"
"net/http/httptest"
"os"
"reflect"
"strings"
"testing"

"go.uber.org/mock/gomock"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/spf13/pflag"
"go.uber.org/mock/gomock"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/client-go/kubernetes"
testclient "k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/rest"
Expand All @@ -33,6 +38,218 @@ func TestIt(t *testing.T) {
RunSpecs(t, "Console Test Suite")
}

func TestConsolePluginV1Alpha1URL(t *testing.T) {
validPlugin := func() unstructured.Unstructured {
return unstructured.Unstructured{Object: map[string]interface{}{
"metadata": map[string]interface{}{"name": "monitoring-plugin"},
"spec": map[string]interface{}{
"service": map[string]interface{}{
"name": "monitoring-plugin",
"namespace": "openshift-monitoring",
"port": int64(9443),
"basePath": "/",
},
},
}}
}

tests := []struct {
name string
mutate func(map[string]interface{})
expectedURL string
expectedField string
expectedKind string
expectWrapped bool
}{
{
name: "valid service with int64 port",
expectedURL: "monitoring-plugin=https://monitoring-plugin.openshift-monitoring.svc.cluster.local:9443/",
},
{
name: "missing service name",
mutate: func(service map[string]interface{}) {
delete(service, "name")
},
expectedField: "spec.service.name",
expectedKind: "missing",
},
{
name: "wrong typed service name",
mutate: func(service map[string]interface{}) {
service["name"] = 42
},
expectedField: "spec.service.name",
expectedKind: "failed to read",
expectWrapped: true,
},
{
name: "missing service namespace",
mutate: func(service map[string]interface{}) {
delete(service, "namespace")
},
expectedField: "spec.service.namespace",
expectedKind: "missing",
},
{
name: "wrong typed service namespace",
mutate: func(service map[string]interface{}) {
service["namespace"] = 42
},
expectedField: "spec.service.namespace",
expectedKind: "failed to read",
expectWrapped: true,
},
{
name: "missing service port",
mutate: func(service map[string]interface{}) {
delete(service, "port")
},
expectedField: "spec.service.port",
expectedKind: "missing",
},
{
name: "wrong typed string service port",
mutate: func(service map[string]interface{}) {
service["port"] = "9443"
},
expectedField: "spec.service.port",
expectedKind: "failed to read",
expectWrapped: true,
},
{
name: "missing service base path",
mutate: func(service map[string]interface{}) {
delete(service, "basePath")
},
expectedField: "spec.service.basePath",
expectedKind: "missing",
},
{
name: "wrong typed service base path",
mutate: func(service map[string]interface{}) {
service["basePath"] = 42
},
expectedField: "spec.service.basePath",
expectedKind: "failed to read",
expectWrapped: true,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
plugin := validPlugin()
if test.mutate != nil {
service := plugin.Object["spec"].(map[string]interface{})["service"].(map[string]interface{})
test.mutate(service)
}

actualURL, err := consolePluginV1Alpha1URL(plugin)
if test.expectedField != "" {
if err == nil {
t.Fatalf("expected error for %s", test.expectedField)
}
for _, expected := range []string{"monitoring-plugin", test.expectedField, test.expectedKind} {
if !strings.Contains(err.Error(), expected) {
t.Errorf("expected error %q to contain %q", err, expected)
}
}
if test.expectWrapped && errors.Unwrap(err) == nil {
t.Errorf("expected error %q to wrap its cause", err)
}
return
}
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if actualURL != test.expectedURL {
t.Fatalf("expected URL %q, got %q", test.expectedURL, actualURL)
}
})
}
}

func TestGetConsolePluginFrom411Cluster(t *testing.T) {
const (
consolePluginsPath = "/apis/console.openshift.io/v1alpha1/consoleplugins"
consoleOperatorPath = "/apis/operator.openshift.io/v1/consoles/cluster"
)

t.Run("lists the expected resource, decodes JSON, and filters disabled plugins", func(t *testing.T) {
requestCounts := map[string]int{}
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
requestCounts[r.URL.Path]++
w.Header().Set("Content-Type", "application/json")
switch r.URL.Path {
case consolePluginsPath:
_, _ = fmt.Fprint(w, `{
"apiVersion":"console.openshift.io/v1alpha1",
"kind":"ConsolePluginList",
"items":[
{"apiVersion":"console.openshift.io/v1alpha1","kind":"ConsolePlugin","metadata":{"name":"enabled-plugin"},"spec":{"service":{"name":"enabled-service","namespace":"enabled-namespace","port":9443,"basePath":"/enabled"}}},
{"apiVersion":"console.openshift.io/v1alpha1","kind":"ConsolePlugin","metadata":{"name":"disabled-plugin"},"spec":{"service":{"name":"disabled-service","namespace":"disabled-namespace","port":9444,"basePath":"/disabled"}}}
]
}`)
case consoleOperatorPath:
_, _ = fmt.Fprint(w, `{"apiVersion":"operator.openshift.io/v1","kind":"Console","metadata":{"name":"cluster"},"spec":{"plugins":["enabled-plugin"]}}`)
default:
http.NotFound(w, r)
}
}))
defer server.Close()

plugins, err := getConsolePluginFrom411Cluster(&rest.Config{Host: server.URL})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
expected := []string{"enabled-plugin=https://enabled-service.enabled-namespace.svc.cluster.local:9443/enabled"}
if !reflect.DeepEqual(plugins, expected) {
t.Fatalf("expected plugins %v, got %v", expected, plugins)
}
if requestCounts[consolePluginsPath] != 1 {
t.Errorf("expected one request to %s, got %d", consolePluginsPath, requestCounts[consolePluginsPath])
}
if requestCounts[consoleOperatorPath] != 2 {
t.Errorf("expected one operator lookup per plugin, got %d", requestCounts[consoleOperatorPath])
}
if len(requestCounts) != 2 {
t.Errorf("unexpected request paths: %v", requestCounts)
}
})

tests := []struct {
name string
failingURL string
}{
{name: "console plugin list API error", failingURL: consolePluginsPath},
{name: "console operator API error", failingURL: consoleOperatorPath},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
if r.URL.Path == test.failingURL {
http.Error(w, `{"kind":"Status","apiVersion":"v1","status":"Failure","message":"injected API error","reason":"InternalError","code":500}`, http.StatusInternalServerError)
return
}
if r.URL.Path != consolePluginsPath {
http.NotFound(w, r)
return
}
_, _ = fmt.Fprint(w, `{"apiVersion":"console.openshift.io/v1alpha1","kind":"ConsolePluginList","items":[{"apiVersion":"console.openshift.io/v1alpha1","kind":"ConsolePlugin","metadata":{"name":"enabled-plugin"},"spec":{"service":{"name":"enabled-service","namespace":"enabled-namespace","port":9443,"basePath":"/"}}}]}`)
}))
defer server.Close()

plugins, err := getConsolePluginFrom411Cluster(&rest.Config{Host: server.URL})
if err == nil {
t.Fatalf("expected API error, got plugins %v", plugins)
}
if !strings.Contains(err.Error(), "injected API error") {
t.Errorf("expected propagated API error, got %v", err)
}
})
}
}

var _ = Describe("console command", func() {
var (
mockCtrl *gomock.Controller
Expand Down
Loading