From 9cb3e0f1c613fc079e3549259c940e170ba0960c Mon Sep 17 00:00:00 2001 From: phantomjinx Date: Mon, 1 Jan 2024 16:55:24 +0000 Subject: [PATCH 1/8] Introduces capabilities module for determining cluster particulars #100 * APICapabilities determines cluster version and existence of certain APIs, eg. Routes. From this determination, it concludes whether the cluster is OpenShift4 compatible * Allows a clear determination as to whether this is an OpenShift cluster or vanilla Kubernetes. This can be used to make choices on what to install, eg. Route or Ingress * Have the cluster capabilities evaluated on initialising of the operator --- pkg/capabilities/capabilities.go | 140 +++++++++ pkg/capabilities/capabilities_test.go | 325 +++++++++++++++++++++ pkg/controller/hawtio/fakeclient_test.go | 25 +- pkg/controller/hawtio/hawtio_controller.go | 62 ++-- 4 files changed, 513 insertions(+), 39 deletions(-) create mode 100644 pkg/capabilities/capabilities.go create mode 100644 pkg/capabilities/capabilities_test.go diff --git a/pkg/capabilities/capabilities.go b/pkg/capabilities/capabilities.go new file mode 100644 index 0000000000..35ecc5a8b8 --- /dev/null +++ b/pkg/capabilities/capabilities.go @@ -0,0 +1,140 @@ +package capabilities + +import ( + "context" + "errors" + "fmt" + + configv1 "github.com/openshift/api/config/v1" + configclient "github.com/openshift/client-go/config/clientset/versioned" + kerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kclient "k8s.io/client-go/kubernetes" + + "github.com/Masterminds/semver" + errs "github.com/pkg/errors" +) + +type ApiServerSpec struct { + Version string // Set to the version of the cluster + KubeVersion string // Set to the kubernetes version of the cluster (different to version if using OpenShift, for example) + IsOpenShift4 bool // Set to true if running on openshift 4 + IsOpenShift43Plus bool // Set to true if running openshift 4.3+ + ImageStreams bool // Set to true if the API Server supports imagestreams + Routes bool // Set to true if the API Server supports routes + ConsoleLink bool // Set to true if the API Server support the openshift console link API +} + +type RequiredApiSpec struct { + routes string + imagestreams string + consolelinks string +} + +var RequiredApi = RequiredApiSpec{ + routes: "routes.route.openshift.io/v1", + imagestreams: "imagestreams.image.openshift.io/v1", + consolelinks: "consolelinks.console.openshift.io/v1", +} + +func contains(a []string, x string) bool { + for _, n := range a { + if x == n { + return true + } + } + return false +} + +func createResourceIndex(apiClient kclient.Interface) ([]string, error) { + _, apiResourceLists, err := apiClient.Discovery().ServerGroupsAndResources() + if err != nil { + return nil, err + } + + resIndex := []string{} + + for _, apiResList := range apiResourceLists { + for _, apiResource := range apiResList.APIResources { + resIndex = append(resIndex, fmt.Sprintf("%s.%s", apiResource.Name, apiResList.GroupVersion)) + } + } + + return resIndex, nil +} + +// APICapabilities For testing the given platform's capabilities +func APICapabilities(ctx context.Context, apiClient kclient.Interface, configClient configclient.Interface) (*ApiServerSpec, error) { + if apiClient == nil { + return nil, errors.New("No api client. Cannot determine api capabilities") + } + + if configClient == nil { + return nil, errors.New("No config client. Cannot determine api capabilities") + } + + apiSpec := ApiServerSpec{} + + info, err := apiClient.Discovery().ServerVersion() + if err != nil { + return nil, errs.Wrap(err, "Failed to discover server version") + } + + apiSpec.KubeVersion = info.Major + "." + info.Minor + + resIndex, err := createResourceIndex(apiClient) + if err != nil { + return nil, errs.Wrap(err, "Failed to create API Resource index") + } + + apiSpec.Routes = contains(resIndex, RequiredApi.routes) + apiSpec.ImageStreams = contains(resIndex, RequiredApi.imagestreams) + apiSpec.ConsoleLink = contains(resIndex, RequiredApi.consolelinks) + + apiSpec.IsOpenShift4 = false + + var clusterVersion *configv1.ClusterVersion + if apiSpec.ConsoleLink { + // + // Update the kubernetes version to the Openshift Version + // + clusterVersion, err = configClient.ConfigV1().ClusterVersions().Get(ctx, "version", metav1.GetOptions{}) + if err == nil { + apiSpec.IsOpenShift4 = true + } else if !kerrors.IsNotFound(err) { + // Some other error rather than not found + // If error is not found then treat as not OpenShift + return nil, errs.Wrap(err, "Error reading cluster version") + } + } + + if apiSpec.IsOpenShift4 && clusterVersion != nil { + // Let's take the latest version from the history + for _, update := range clusterVersion.Status.History { + if update.State == configv1.CompletedUpdate { + // Obtain the version from the last completed update + // Update the api spec version + var openShiftSemVer *semver.Version + openShiftSemVer, err = semver.NewVersion(update.Version) + apiSpec.Version = openShiftSemVer.String() + + // Update whether this is OpenShift 4.3+ + constraint43, _ := semver.NewConstraint(">= 4.3") + apiSpec.IsOpenShift43Plus = constraint43.Check(openShiftSemVer) + if err != nil { + return nil, errs.Wrap(err, fmt.Sprintf("Error parsing OpenShift cluster semantic version %s", update.Version)) + } + break + } + } + } else { + // This is not OpenShift so plain kubernetes or something else + apiSpec.IsOpenShift4 = false + + // Update version to kubernetes version + apiSpec.Version = apiSpec.KubeVersion + apiSpec.IsOpenShift43Plus = false + } + + return &apiSpec, nil +} diff --git a/pkg/capabilities/capabilities_test.go b/pkg/capabilities/capabilities_test.go new file mode 100644 index 0000000000..12571cc58c --- /dev/null +++ b/pkg/capabilities/capabilities_test.go @@ -0,0 +1,325 @@ +/* + * Copyright (C) 2019 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package capabilities + +import ( + "context" + "testing" + "time" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/version" + + configv1 "github.com/openshift/api/config/v1" + fakeconfig "github.com/openshift/client-go/config/clientset/versioned/fake" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + fakediscovery "k8s.io/client-go/discovery/fake" + fakekube "k8s.io/client-go/kubernetes/fake" +) + +func Test_ApiCapabilities(t *testing.T) { + + res1 := metav1.APIResourceList{ + GroupVersion: "image.openshift.io/v1", + APIResources: []metav1.APIResource{ + {Name: "imagestreams"}, + }, + } + res2 := metav1.APIResourceList{ + GroupVersion: "route.openshift.io/v1", + APIResources: []metav1.APIResource{ + {Name: "routes"}, + }, + } + res3 := metav1.APIResourceList{ + GroupVersion: "oauth.openshift.io/v1", + APIResources: []metav1.APIResource{ + {Name: "oauthclientauthorizations"}, + }, + } + res3a := metav1.APIResourceList{ + GroupVersion: "console.openshift.io/v1", + APIResources: []metav1.APIResource{ + {Name: "consolelinks"}, + }, + } + + res4 := metav1.APIResourceList{ + GroupVersion: "something.openshift.io/v1", + } + res5 := metav1.APIResourceList{ + GroupVersion: "not.anything.io/v1", + } + res6 := metav1.APIResourceList{ + GroupVersion: "something.else.io/v1", + } + + startTime, _ := time.Parse(time.RFC3339, "2023-09-18T15:35:39Z") + + clusterVersion := &configv1.ClusterVersion{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterVersion", + APIVersion: "config.openshift.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID("86c5af53-0a82-4d59-957a-4c5a74caf26d"), + }, + Status: configv1.ClusterVersionStatus{ + Desired: configv1.Release{ + Image: "quay.io/crcont/ocp-release@sha256:4769b0a12fbbd7c2d6846a6bcf69761a1339d39bd96be8d44c122eac032caad1", + Version: "4.13.12", + }, + History: []configv1.UpdateHistory{ + { + State: configv1.CompletedUpdate, + StartedTime: metav1.NewTime(startTime), + CompletionTime: nil, + Image: "quay.io/crcont/ocp-release@sha256:4769b0a12fbbd7c2d6846a6bcf69761a1339d39bd96be8d44c122eac032caad1", + Verified: false, + Version: "4.13.12", + }, + }, + }, + } + + testCases := []struct { + name string + resList []*metav1.APIResourceList + expected ApiServerSpec + osversion *configv1.ClusterVersion + }{ + { + "Relevant APIs available for fully true api spec", + []*metav1.APIResourceList{&res1, &res2, &res3, &res3a}, + ApiServerSpec{ + Version: "4.13.12", + KubeVersion: "1.26", + IsOpenShift4: true, + IsOpenShift43Plus: true, + ImageStreams: true, + Routes: true, + ConsoleLink: true, + }, + clusterVersion, + }, + { + "No relevant resources so expect false", + []*metav1.APIResourceList{&res4, &res5, &res6}, + ApiServerSpec{ + Version: "1.26", + KubeVersion: "1.26", + IsOpenShift4: false, + IsOpenShift43Plus: false, + ImageStreams: false, + Routes: false, + }, + nil, + }, + { + "No resources so everything false", + []*metav1.APIResourceList{}, + ApiServerSpec{ + Version: "1.26", + KubeVersion: "1.26", + IsOpenShift4: false, + IsOpenShift43Plus: false, + ImageStreams: false, + Routes: false, + }, + nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + api := fakekube.NewSimpleClientset() + fd := api.Discovery().(*fakediscovery.FakeDiscovery) + fd.Resources = tc.resList + fd.FakedServerVersion = &version.Info{ + Major: "1", + Minor: "26", + } + + var configObjects []runtime.Object + if tc.osversion != nil { + configObjects = append(configObjects, tc.osversion) + } + configClient := fakeconfig.NewSimpleClientset(configObjects...) + + apiSpec, err := APICapabilities(context.TODO(), api, configClient) + if err != nil { + t.Error(err) + } + + if apiSpec == nil { + t.Error("Failed to return an api specification") + } + + if apiSpec.Version != tc.expected.Version { + t.Error("Expected api specification version not expected", "actual", apiSpec.Version, "expected", tc.expected.Version) + } + + if apiSpec.KubeVersion != tc.expected.KubeVersion { + t.Error("Not Expected kube version", "actual", apiSpec.KubeVersion, "expected", tc.expected.KubeVersion) + } + + if apiSpec.IsOpenShift4 != tc.expected.IsOpenShift4 { + t.Error("Not Expected cluster to be openshift4", "actual", apiSpec.IsOpenShift4, "expected", tc.expected.IsOpenShift4) + } + + if apiSpec.Routes != tc.expected.Routes { + t.Error("Expected api specification routes not expected") + } + + if apiSpec.ImageStreams != tc.expected.ImageStreams { + t.Error("Expected api specification image streams not expected") + } + }) + } +} + +func Test_ApiCapabilitiesOpenshift3(t *testing.T) { + + res1 := metav1.APIResourceList{ + GroupVersion: "image.openshift.io/v1", + APIResources: []metav1.APIResource{ + {Name: "imagestreams"}, + }, + } + + testCases := []struct { + name string + resList []*metav1.APIResourceList + expected ApiServerSpec + }{ + { + "Openshift 3 parse version 1.11", + []*metav1.APIResourceList{&res1}, + ApiServerSpec{ + Version: "1.11", + KubeVersion: "1.11", + IsOpenShift4: false, + IsOpenShift43Plus: false, + ImageStreams: true, + Routes: true, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + api := fakekube.NewSimpleClientset() + configClient := fakeconfig.NewSimpleClientset() + fd := api.Discovery().(*fakediscovery.FakeDiscovery) + fd.Resources = tc.resList + fd.FakedServerVersion = &version.Info{ + Major: "1", + Minor: "11", + } + + apiSpec, err := APICapabilities(context.TODO(), api, configClient) + if err != nil { + t.Error(err) + } + + if apiSpec == nil { + t.Error("Failed to return an api specification") + } + + if apiSpec.Version != tc.expected.Version { + t.Error("Expected api specification version not expected", "Actual", apiSpec.Version, "Expected", tc.expected.Version) + } + + if apiSpec.KubeVersion != tc.expected.KubeVersion { + t.Error("Not Expected kube version", "actual", apiSpec.KubeVersion, "expected", tc.expected.KubeVersion) + } + + if apiSpec.IsOpenShift4 { + t.Error("Expected not to be openshift cluster") + } + + if apiSpec.ImageStreams != tc.expected.ImageStreams { + t.Error("Expected api specification image streams not expected") + } + + }) + } +} + +func Test_ApiCapabilitiesKubernetes(t *testing.T) { + + testCases := []struct { + name string + resList []*metav1.APIResourceList + expected ApiServerSpec + }{ + { + "Vanilla Kubernetes parse version 1.26", + []*metav1.APIResourceList{}, + ApiServerSpec{ + Version: "1.26", + KubeVersion: "1.26", + IsOpenShift4: false, + IsOpenShift43Plus: false, + ImageStreams: false, + Routes: false, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + api := fakekube.NewSimpleClientset() + configClient := fakeconfig.NewSimpleClientset() + fd := api.Discovery().(*fakediscovery.FakeDiscovery) + fd.Resources = tc.resList + fd.FakedServerVersion = &version.Info{ + Major: "1", + Minor: "26", + } + + apiSpec, err := APICapabilities(context.TODO(), api, configClient) + if err != nil { + t.Error(err) + } + + if apiSpec == nil { + t.Error("Failed to return an api specification") + } + + if apiSpec.Version != tc.expected.Version { + t.Error("Expected api specification version not expected", "Actual", apiSpec.Version, "Expected", tc.expected.Version) + } + + if apiSpec.KubeVersion != tc.expected.KubeVersion { + t.Error("Not Expected kube version", "actual", apiSpec.KubeVersion, "expected", tc.expected.KubeVersion) + } + + if apiSpec.IsOpenShift4 { + t.Error("Expected not to be openshift cluster") + } + + if apiSpec.ImageStreams != tc.expected.ImageStreams { + t.Error("Expected api specification image streams not expected") + } + + }) + } +} diff --git a/pkg/controller/hawtio/fakeclient_test.go b/pkg/controller/hawtio/fakeclient_test.go index e4cb792486..87de5d37c2 100644 --- a/pkg/controller/hawtio/fakeclient_test.go +++ b/pkg/controller/hawtio/fakeclient_test.go @@ -1,6 +1,7 @@ package hawtio import ( + "context" "testing" "github.com/stretchr/testify/assert" @@ -8,6 +9,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/version" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -15,8 +17,11 @@ import ( fakeconfig "github.com/openshift/client-go/config/clientset/versioned/fake" fakeoauth "github.com/openshift/client-go/oauth/clientset/versioned/fake" + discoveryfake "k8s.io/client-go/discovery/fake" + fakekube "k8s.io/client-go/kubernetes/fake" "github.com/hawtio/hawtio-operator/pkg/apis" + "github.com/hawtio/hawtio-operator/pkg/capabilities" ) //buildReconcileWithFakeClientWithMocks return *ReconcileHawtio with fake client, scheme and mock objects @@ -44,11 +49,29 @@ func buildReconcileWithFakeClientWithMocks(objs []runtime.Object, t *testing.T) } client := fake.NewFakeClientWithScheme(scheme, objs...) + apiClient := fakekube.NewSimpleClientset() + + fd := apiClient.Discovery().(*discoveryfake.FakeDiscovery) + fd.FakedServerVersion = &version.Info{ + Major: "4", + Minor: "13", + } + + configClient := fakeconfig.NewSimpleClientset() + coreClient := apiClient.CoreV1() + + apiSpec, err := capabilities.APICapabilities(context.TODO(), apiClient, configClient) + if err != nil { + assert.Fail(t, "unable to define api capabilities") + } return &ReconcileHawtio{ scheme: scheme, client: client, - configClient: fakeconfig.NewSimpleClientset(), + configClient: configClient, + coreClient: coreClient, oauthClient: fakeoauth.NewSimpleClientset(), + apiClient: apiClient, + apiSpec: apiSpec, } } diff --git a/pkg/controller/hawtio/hawtio_controller.go b/pkg/controller/hawtio/hawtio_controller.go index 97bc4a5b6e..1d36a796ab 100644 --- a/pkg/controller/hawtio/hawtio_controller.go +++ b/pkg/controller/hawtio/hawtio_controller.go @@ -9,8 +9,6 @@ import ( "time" - "github.com/Masterminds/semver" - "github.com/RHsyseng/operator-utils/pkg/resource/compare" "github.com/RHsyseng/operator-utils/pkg/resource/read" "github.com/RHsyseng/operator-utils/pkg/resource/write" @@ -24,6 +22,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + kclient "k8s.io/client-go/kubernetes" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -43,6 +42,7 @@ import ( oauthclient "github.com/openshift/client-go/oauth/clientset/versioned" hawtiov1 "github.com/hawtio/hawtio-operator/pkg/apis/hawtio/v1" + "github.com/hawtio/hawtio-operator/pkg/capabilities" "github.com/hawtio/hawtio-operator/pkg/openshift" "github.com/hawtio/hawtio-operator/pkg/resources" "github.com/hawtio/hawtio-operator/pkg/util" @@ -93,11 +93,18 @@ func Add(mgr manager.Manager, bv util.BuildVariables) error { } r.configClient = configClient - coreClient, err := corev1client.NewForConfig(mgr.GetConfig()) + apiClient, err := kclient.NewForConfig(mgr.GetConfig()) if err != nil { return err } - r.coreClient = coreClient + r.apiClient = apiClient + r.coreClient = apiClient.CoreV1() + + // Identify cluster capabilities + r.apiSpec, err = capabilities.APICapabilities(context.TODO(), r.apiClient, r.configClient) + if err != nil { + return errs.Wrap(err, "Cluster API capability discovery failed") + } if err := openshift.ConsoleYAMLSampleExists(); err == nil { openshift.CreateConsoleYAMLSamples(context.TODO(), mgr.GetClient(), r.ProductName) @@ -177,9 +184,11 @@ type ReconcileHawtio struct { // that reads objects from the cache and writes to the API server client client.Client scheme *runtime.Scheme - coreClient *corev1client.CoreV1Client + coreClient corev1client.CoreV1Interface oauthClient oauthclient.Interface configClient configclient.Interface + apiClient kclient.Interface + apiSpec *capabilities.ApiServerSpec } // Reconcile reads that state of the cluster for a Hawtio object and makes changes based on the state read @@ -205,6 +214,8 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque return reconcile.Result{}, err } + reqLogger.Info(fmt.Sprintf("Cluster API Specification: %+v", r.apiSpec)) + // Delete phase if hawtio.GetDeletionTimestamp() != nil { @@ -262,33 +273,8 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque return reconcile.Result{Requeue: true}, nil } - // Check OpenShift version - var openShiftSemVer *semver.Version - clusterVersion, err := r.configClient.ConfigV1().ClusterVersions().Get(ctx, "version", metav1.GetOptions{}) - if err != nil { - if errors.IsNotFound(err) { - // Let's default to OpenShift 3 as ClusterVersion API was introduced in OpenShift 4 - openShiftSemVer, _ = semver.NewVersion("3") - } else { - reqLogger.Error(err, "Error reading cluster version") - return reconcile.Result{}, err - } - } else { - // Let's take the latest version from the history - v := clusterVersion.Status.History[0].Version - openShiftSemVer, err = semver.NewVersion(v) - if err != nil { - reqLogger.Error(err, "Error parsing cluster semantic version", "version", v) - return reconcile.Result{}, err - } - } - constraint4, _ := semver.NewConstraint(">= 4-0") - isOpenShift4 := constraint4.Check(openShiftSemVer) - constraint43, _ := semver.NewConstraint(">= 4.3") - isOpenShift43Plus := constraint43.Check(openShiftSemVer) - var openShiftConsoleUrl string - if isOpenShift4 { + if r.apiSpec.IsOpenShift4 { // Retrieve OpenShift Web console public URL cm, err := r.coreClient.ConfigMaps("openshift-config-managed").Get(ctx, "console-public", metav1.GetOptions{}) if err != nil { @@ -301,7 +287,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque } } - if isOpenShift4 { + if r.apiSpec.IsOpenShift4 { cronJob := &batchv1.CronJob{} cronJobName := request.Name + "-certificate-expiry-check" cronJobErr := r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: cronJobName}, cronJob) @@ -443,7 +429,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque caCertRouteSecret = nil } - _, err = r.reconcileDeployment(hawtio, isOpenShift4, openShiftSemVer.String(), openShiftConsoleUrl, + _, err = r.reconcileDeployment(hawtio, r.apiSpec.IsOpenShift4, r.apiSpec.Version, openShiftConsoleUrl, configMap, clientCertSecret, tlsRouteSecret, caCertRouteSecret) if err != nil { reqLogger.Error(err, "Error reconciling deployment") @@ -477,7 +463,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque // Add link to OpenShift console consoleLinkName := request.Name + "-" + request.Namespace - if isOpenShift4 && hawtio.Status.Phase == hawtiov1.HawtioPhaseInitialized { + if r.apiSpec.IsOpenShift4 && hawtio.Status.Phase == hawtiov1.HawtioPhaseInitialized { consoleLink := &consolev1.ConsoleLink{} err = r.client.Get(ctx, types.NamespacedName{Name: consoleLinkName}, consoleLink) if err != nil { @@ -495,7 +481,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque consoleLink = &consolev1.ConsoleLink{} if isClusterDeployment { consoleLink = openshift.NewApplicationMenuLink(consoleLinkName, route, hawtconfig) - } else if isOpenShift43Plus { + } else if r.apiSpec.IsOpenShift43Plus { consoleLink = openshift.NewNamespaceDashboardLink(consoleLinkName, request.Namespace, route, hawtconfig) } if consoleLink.Spec.Location != "" { @@ -563,7 +549,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque } // Reconcile console link in OpenShift console - if isOpenShift4 { + if r.apiSpec.IsOpenShift4 { consoleLink := &consolev1.ConsoleLink{} err = r.client.Get(ctx, types.NamespacedName{Name: consoleLinkName}, consoleLink) if err != nil { @@ -571,7 +557,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque // If not found, create a console link if isClusterDeployment { consoleLink = openshift.NewApplicationMenuLink(consoleLinkName, route, hawtconfig) - } else if isOpenShift43Plus { + } else if r.apiSpec.IsOpenShift43Plus { consoleLink = openshift.NewNamespaceDashboardLink(consoleLinkName, request.Namespace, route, hawtconfig) } if consoleLink.Spec.Location != "" { @@ -589,7 +575,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque consoleLinkCopy := consoleLink.DeepCopy() if isClusterDeployment { openshift.UpdateApplicationMenuLink(consoleLinkCopy, route, hawtconfig) - } else if isOpenShift43Plus { + } else if r.apiSpec.IsOpenShift43Plus { openshift.UpdateNamespaceDashboardLink(consoleLinkCopy, route, hawtconfig) } err = r.client.Patch(ctx, consoleLinkCopy, client.MergeFrom(consoleLink)) From 97d25f729b1789252f6916ccfbad03b8142244c5 Mon Sep 17 00:00:00 2001 From: phantomjinx Date: Tue, 9 Jan 2024 20:15:25 +0000 Subject: [PATCH 2/8] Start to modify the watches to evaluate what is present in the cluster #100 * Only evaluate the Route API if it is present in the cluster. --- pkg/controller/hawtio/hawtio_controller.go | 36 ++++++++++++++++------ 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/pkg/controller/hawtio/hawtio_controller.go b/pkg/controller/hawtio/hawtio_controller.go index 1d36a796ab..fa476d88ab 100644 --- a/pkg/controller/hawtio/hawtio_controller.go +++ b/pkg/controller/hawtio/hawtio_controller.go @@ -2,20 +2,22 @@ package hawtio import ( "context" + "errors" "fmt" "os" "reflect" "strings" "time" - "github.com/RHsyseng/operator-utils/pkg/resource/compare" "github.com/RHsyseng/operator-utils/pkg/resource/read" "github.com/RHsyseng/operator-utils/pkg/resource/write" + errs "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -110,15 +112,15 @@ func Add(mgr manager.Manager, bv util.BuildVariables) error { openshift.CreateConsoleYAMLSamples(context.TODO(), mgr.GetClient(), r.ProductName) } - return add(mgr, r) + return add(mgr, r, r.apiSpec.Routes) } // add adds a new Controller to mgr with r as the reconcile.Reconciler -func add(mgr manager.Manager, r reconcile.Reconciler) error { +func add(mgr manager.Manager, r reconcile.Reconciler, routeSupport bool) error { // Create a new controller c, err := controller.New("hawtio-controller", mgr, controller.Options{Reconciler: r}) if err != nil { - return err + return errs.Wrap(err, "Failed to create new controller") } // Watch for changes to primary resource Hawtio @@ -133,7 +135,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { }, }) if err != nil { - return err + return errs.Wrap(err, "Failed to create watch for Hawtio resource") } // Watch for changes to secondary resources and requeue the owner Hawtio @@ -142,15 +144,27 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { OwnerType: &hawtiov1.Hawtio{}, }) if err != nil { - return err + return errs.Wrap(err, "Failed to create watch for ConfigMap resource") + } + + if routeSupport { + err = c.Watch(&source.Kind{Type: &routev1.Route{}}, &handler.EnqueueRequestForOwner{ + IsController: true, + OwnerType: &hawtiov1.Hawtio{}, + }) + if err != nil { + return errs.Wrap(err, "Failed to create watch for Route resource") + } } - err = c.Watch(&source.Kind{Type: &routev1.Route{}}, &handler.EnqueueRequestForOwner{ + + err = c.Watch(&source.Kind{Type: &networkingv1.Ingress{}}, &handler.EnqueueRequestForOwner{ IsController: true, OwnerType: &hawtiov1.Hawtio{}, }) if err != nil { - return err + return errs.Wrap(err, "Failed to create watch for Ingress resource") } + err = c.Watch(&source.Kind{Type: &appsv1.Deployment{}}, &handler.EnqueueRequestForOwner{ IsController: true, OwnerType: &hawtiov1.Hawtio{}, @@ -163,13 +177,17 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { return oldDeployment.Status.Replicas != newDeployment.Status.Replicas }, }) + if err != nil { + return errs.Wrap(err, "Failed to create watch for Deployment resource") + } + //watch secret err = c.Watch(&source.Kind{Type: &corev1.Secret{}}, &handler.EnqueueRequestForOwner{ IsController: true, OwnerType: &hawtiov1.Hawtio{}, }) if err != nil { - return err + return errs.Wrap(err, "Failed to create watch for Secret resource") } return nil From 3d16b4df7810421358d190dbe426b36a336d21ef Mon Sep 17 00:00:00 2001 From: phantomjinx Date: Tue, 9 Jan 2024 20:16:44 +0000 Subject: [PATCH 3/8] Start to wrap variables into struct properties #100 * hawtio_controller.go * Rather than adding to the extensive list of function parameters, roll them up into a single struct that can be populated then passed around instead * deployment.go * Pass in the capabilities.ApiSpec that wraps up both the IsOpenShift and version properties --- pkg/controller/hawtio/hawtio_controller.go | 73 ++++++++++++---------- pkg/resources/deployment.go | 19 +++--- 2 files changed, 50 insertions(+), 42 deletions(-) diff --git a/pkg/controller/hawtio/hawtio_controller.go b/pkg/controller/hawtio/hawtio_controller.go index fa476d88ab..81a1390b8f 100644 --- a/pkg/controller/hawtio/hawtio_controller.go +++ b/pkg/controller/hawtio/hawtio_controller.go @@ -209,6 +209,15 @@ type ReconcileHawtio struct { apiSpec *capabilities.ApiServerSpec } +// DeploymentConfiguration acquires properties used in deployment +type DeploymentConfiguration struct { + openShiftConsoleURL string + configMap *corev1.ConfigMap + clientCertSecret *corev1.Secret + tlsRouteSecret *corev1.Secret + caCertRouteSecret *corev1.Secret +} + // Reconcile reads that state of the cluster for a Hawtio object and makes changes based on the state read // and what is in the Hawtio.Spec // Note: @@ -232,6 +241,10 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque return reconcile.Result{}, err } + deploymentConfig := DeploymentConfiguration{} + // This secret name should be the same as used in deployment.go + clientSecretName := hawtio.Name + "-tls-proxying" + reqLogger.Info(fmt.Sprintf("Cluster API Specification: %+v", r.apiSpec)) // Delete phase @@ -291,7 +304,6 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque return reconcile.Result{Requeue: true}, nil } - var openShiftConsoleUrl string if r.apiSpec.IsOpenShift4 { // Retrieve OpenShift Web console public URL cm, err := r.coreClient.ConfigMaps("openshift-config-managed").Get(ctx, "console-public", metav1.GetOptions{}) @@ -301,7 +313,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque return reconcile.Result{}, err } } else { - openShiftConsoleUrl = cm.Data["consoleURL"] + deploymentConfig.openShiftConsoleURL = cm.Data["consoleURL"] } } @@ -311,13 +323,12 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque cronJobErr := r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: cronJobName}, cronJob) // Check whether client certificate secret exists - secretName := request.Name + "-tls-proxying" clientCertSecret := corev1.Secret{} - err := r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: secretName}, &clientCertSecret) + err := r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: clientSecretName}, &clientCertSecret) // TODO: Update the client certificate when the Hawtio resource changes if err != nil { if errors.IsNotFound(err) { - reqLogger.Info("Client certificate secret not found, creating a new one", "secret", secretName) + reqLogger.Info("Client certificate secret not found, creating a new one", "secret", clientSecretName) caSecret, err := r.coreClient.Secrets("openshift-service-ca").Get(ctx, "signing-key", metav1.GetOptions{}) if err != nil { @@ -338,7 +349,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque if date := hawtio.Spec.Auth.ClientCertExpirationDate; date != nil && !date.IsZero() { expirationDate = date.Time } - certSecret, err := generateCertificateSecret(secretName, request.Namespace, caSecret, commonName, expirationDate) + certSecret, err := generateCertificateSecret(clientSecretName, request.Namespace, caSecret, commonName, expirationDate) if err != nil { reqLogger.Error(err, "Generating the client certificate failed") return reconcile.Result{}, err @@ -348,7 +359,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque return reconcile.Result{}, err } _, err = r.coreClient.Secrets(request.Namespace).Create(ctx, certSecret, metav1.CreateOptions{}) - reqLogger.Info("Client certificate created successfully", "secret", secretName) + reqLogger.Info("Client certificate created successfully", "secret", clientSecretName) if err != nil { reqLogger.Error(err, "Creating the client certificate secret failed") return reconcile.Result{}, err @@ -413,42 +424,40 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque return reconcile.Result{}, err } - configMap := &corev1.ConfigMap{} - err = r.client.Get(ctx, request.NamespacedName, configMap) + deploymentConfig.configMap = &corev1.ConfigMap{} + err = r.client.Get(ctx, request.NamespacedName, deploymentConfig.configMap) if err != nil { reqLogger.Error(err, "Failed to get config map") return reconcile.Result{}, err } - secretName := request.Name + "-tls-proxying" - var clientCertSecret = &corev1.Secret{} - err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: secretName}, clientCertSecret) + deploymentConfig.clientCertSecret = &corev1.Secret{} + err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: clientSecretName}, deploymentConfig.clientCertSecret) if err != nil { - // clientCertSecret isn't used on openshift 3 - clientCertSecret = nil + // clientCertSecret is only used on OpenShift + deploymentConfig.clientCertSecret = nil } - tlsRouteSecret := &corev1.Secret{} + // + // Custom Route certificates defined in Hawtio CR + // if secretName := hawtio.Spec.Route.CertSecret.Name; secretName != "" { - err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: secretName}, tlsRouteSecret) + deploymentConfig.tlsRouteSecret = &corev1.Secret{} + err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: secretName}, deploymentConfig.tlsRouteSecret) if err != nil { return reconcile.Result{}, err } - } else { - tlsRouteSecret = nil } - caCertRouteSecret := &corev1.Secret{} + if caCertSecretName := hawtio.Spec.Route.CaCert.Name; caCertSecretName != "" { - err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: caCertSecretName}, caCertRouteSecret) + deploymentConfig.caCertRouteSecret = &corev1.Secret{} + err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: caCertSecretName}, deploymentConfig.caCertRouteSecret) if err != nil { return reconcile.Result{}, err } - } else { - caCertRouteSecret = nil } - _, err = r.reconcileDeployment(hawtio, r.apiSpec.IsOpenShift4, r.apiSpec.Version, openShiftConsoleUrl, - configMap, clientCertSecret, tlsRouteSecret, caCertRouteSecret) + _, err = r.reconcileDeployment(hawtio, deploymentConfig) if err != nil { reqLogger.Error(err, "Error reconciling deployment") return reconcile.Result{}, err @@ -473,7 +482,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque } // Read Hawtio configuration - hawtconfig, err := resources.GetHawtioConfig(configMap) + hawtconfig, err := resources.GetHawtioConfig(deploymentConfig.configMap) if err != nil { reqLogger.Error(err, "Failed to get hawtconfig") return reconcile.Result{}, err @@ -697,21 +706,19 @@ func (r *ReconcileHawtio) reconcileConfigMap(hawtio *hawtiov1.Hawtio) (bool, err return r.reconcileResources(hawtio, []client.Object{configMap}, []client.ObjectList{&corev1.ConfigMapList{}}) } -func (r *ReconcileHawtio) reconcileDeployment(hawtio *hawtiov1.Hawtio, - isOpenShift4 bool, openShiftVersion string, openShiftConsoleURL string, - configMap *corev1.ConfigMap, clientCertSecret, tlsCustomSecret, caCertRouteSecret *corev1.Secret) (bool, error) { +func (r *ReconcileHawtio) reconcileDeployment(hawtio *hawtiov1.Hawtio, deploymentConfig DeploymentConfiguration) (bool, error) { clientCertSecretVersion := "" - if clientCertSecret != nil { - clientCertSecretVersion = clientCertSecret.GetResourceVersion() + if deploymentConfig.clientCertSecret != nil { + clientCertSecretVersion = deploymentConfig.clientCertSecret.GetResourceVersion() } - deployment, err := resources.NewDeployment(hawtio, isOpenShift4, openShiftVersion, openShiftConsoleURL, - configMap.GetResourceVersion(), clientCertSecretVersion, r.BuildVariables) + deployment, err := resources.NewDeployment(hawtio, r.apiSpec, deploymentConfig.openShiftConsoleURL, + deploymentConfig.configMap.GetResourceVersion(), clientCertSecretVersion, r.BuildVariables) if err != nil { return false, err } service := resources.NewService(hawtio) - route := resources.NewRoute(hawtio, tlsCustomSecret, caCertRouteSecret) + route := resources.NewRoute(hawtio, deploymentConfig.tlsRouteSecret, deploymentConfig.caCertRouteSecret) var serviceAccount *corev1.ServiceAccount if hawtio.Spec.Type == hawtiov1.NamespaceHawtioDeploymentType { diff --git a/pkg/resources/deployment.go b/pkg/resources/deployment.go index db16fc9133..54fc816661 100644 --- a/pkg/resources/deployment.go +++ b/pkg/resources/deployment.go @@ -14,6 +14,7 @@ import ( "k8s.io/utils/pointer" hawtiov1 "github.com/hawtio/hawtio-operator/pkg/apis/hawtio/v1" + "github.com/hawtio/hawtio-operator/pkg/capabilities" "github.com/hawtio/hawtio-operator/pkg/util" ) @@ -33,8 +34,8 @@ const ( serverRootDirectory = "/usr/share/nginx/html" ) -func NewDeployment(hawtio *hawtiov1.Hawtio, isOpenShift4 bool, openShiftVersion string, openShiftConsoleURL string, configMapVersion string, clientCertSecretVersion string, buildVariables util.BuildVariables) (*appsv1.Deployment, error) { - podTemplateSpec, err := newPodTemplateSpec(hawtio, isOpenShift4, openShiftVersion, openShiftConsoleURL, configMapVersion, clientCertSecretVersion, buildVariables) +func NewDeployment(hawtio *hawtiov1.Hawtio, apiSpec *capabilities.ApiServerSpec, openShiftConsoleURL string, configMapVersion string, clientCertSecretVersion string, buildVariables util.BuildVariables) (*appsv1.Deployment, error) { + podTemplateSpec, err := newPodTemplateSpec(hawtio, apiSpec, openShiftConsoleURL, configMapVersion, clientCertSecretVersion, buildVariables) if err != nil { return nil, err } @@ -73,9 +74,9 @@ func newDeployment(hawtio *hawtiov1.Hawtio, replicas *int32, pts corev1.PodTempl } } -func newPodTemplateSpec(hawtio *hawtiov1.Hawtio, isOpenShift4 bool, openShiftVersion string, openShiftConsoleURL string, configMapVersion string, clientCertSecretVersion string, buildVariables util.BuildVariables) (corev1.PodTemplateSpec, error) { +func newPodTemplateSpec(hawtio *hawtiov1.Hawtio, apiSpec *capabilities.ApiServerSpec, openShiftConsoleURL string, configMapVersion string, clientCertSecretVersion string, buildVariables util.BuildVariables) (corev1.PodTemplateSpec, error) { hawtioVersion := getVersion(buildVariables) - container := newContainer(hawtio, newEnvVars(hawtio, isOpenShift4, openShiftVersion, openShiftConsoleURL), hawtioVersion, buildVariables.ImageRepository) + container := newContainer(hawtio, newEnvVars(hawtio, apiSpec, openShiftConsoleURL), hawtioVersion, buildVariables.ImageRepository) annotations := map[string]string{ configVersionAnnotation: configMapVersion, @@ -85,14 +86,14 @@ func newPodTemplateSpec(hawtio *hawtiov1.Hawtio, isOpenShift4 bool, openShiftVer } propagateAnnotations(hawtio, annotations) - volumeMounts, err := newVolumeMounts(isOpenShift4, hawtioVersion, hawtio.Spec.RBAC.ConfigMap, buildVariables) + volumeMounts, err := newVolumeMounts(apiSpec.IsOpenShift4, hawtioVersion, hawtio.Spec.RBAC.ConfigMap, buildVariables) if err != nil { return corev1.PodTemplateSpec{}, err } if len(volumeMounts) > 0 { container.VolumeMounts = volumeMounts } - volumes := newVolumes(hawtio, isOpenShift4) + volumes := newVolumes(hawtio, apiSpec.IsOpenShift4) labels := labelsForHawtio(hawtio.Name) additionalLabels, err := labelUtils.ConvertSelectorToLabelsMap(buildVariables.AdditionalLabels) @@ -147,14 +148,14 @@ func newVolumes(hawtio *hawtiov1.Hawtio, isOpenShift4 bool) []corev1.Volume { return volumes } -func newEnvVars(hawtio *hawtiov1.Hawtio, isOpenShift4 bool, openShiftVersion string, openShiftConsoleURL string) []corev1.EnvVar { +func newEnvVars(hawtio *hawtiov1.Hawtio, apiSpec *capabilities.ApiServerSpec, openShiftConsoleURL string) []corev1.EnvVar { var envVars []corev1.EnvVar envVarsForHawtio := envVarsForHawtio(hawtio.Spec.Type, hawtio.Name) envVars = append(envVars, envVarsForHawtio...) - if isOpenShift4 { - envVarsForOpenShift4 := envVarsForOpenshift4(openShiftVersion, openShiftConsoleURL) + if apiSpec.IsOpenShift4 { + envVarsForOpenShift4 := envVarsForOpenshift4(apiSpec.Version, openShiftConsoleURL) envVars = append(envVars, envVarsForOpenShift4...) } From c85ed6b9313762d4b29b894c3794c2d2a9d0793c Mon Sep 17 00:00:00 2001 From: phantomjinx Date: Wed, 3 Jan 2024 15:53:18 +0000 Subject: [PATCH 4/8] Modifies the certificate function API to clarify its purpose #100 * The function generateCertificateSecret can be used for both OpenShift CA signing and self-signing certificates. Just a case of determining what is used as both the caCertificate and caPrivateKey (self-signing uses the generated certificate and private key) * Rather than using the same function with the caSecret as nil, makes it obvious which to use by providing front functions that describe which sort of certificate secret will be created install-operator.sh --- pkg/controller/hawtio/certificate.go | 64 ++++++++++++++++------ pkg/controller/hawtio/hawtio_controller.go | 2 +- 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/pkg/controller/hawtio/certificate.go b/pkg/controller/hawtio/certificate.go index 174e90f076..705df96045 100644 --- a/pkg/controller/hawtio/certificate.go +++ b/pkg/controller/hawtio/certificate.go @@ -1,6 +1,7 @@ package hawtio import ( + "crypto" "crypto/rand" "crypto/rsa" "crypto/x509" @@ -18,25 +19,44 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func generateCertificateSecret(name string, namespace string, caSecret *corev1.Secret, commonName string, expirationDate time.Time) (*corev1.Secret, error) { - caCertFile := caSecret.Data["tls.crt"] - pemBlock, _ := pem.Decode(caCertFile) - if pemBlock == nil { - return nil, errors.New("failed to decode CA certificate") - } - caCert, err := x509.ParseCertificate(pemBlock.Bytes) - if err != nil { - return nil, err - } - caKey := caSecret.Data["tls.key"] - pemBlock, _ = pem.Decode(caKey) - if pemBlock == nil { - return nil, errors.New("failed to decode CA certificate signing key") +func generateSelfSignedCertSecret(name string, namespace string, commonName string, expirationDate time.Time) (*corev1.Secret, error) { + return generateCertificateSecret(name, namespace, nil, commonName, expirationDate) +} + +func generateCASignedCertSecret(name string, namespace string, caSecret *corev1.Secret, commonName string, expirationDate time.Time) (*corev1.Secret, error) { + if caSecret == nil { + return nil, errors.New("Generating a CA-signed certificate requires the CA Secret") } - caPrivateKey, err := x509.ParsePKCS1PrivateKey(pemBlock.Bytes) - if err != nil { - return nil, err + + return generateCertificateSecret(name, namespace, caSecret, commonName, expirationDate) +} + +func generateCertificateSecret(name string, namespace string, caSecret *corev1.Secret, commonName string, expirationDate time.Time) (*corev1.Secret, error) { + var caCert *x509.Certificate + var caPrivateKey crypto.PrivateKey + var err error + + if caSecret != nil { + caCertFile := caSecret.Data["tls.crt"] + pemBlock, _ := pem.Decode(caCertFile) + if pemBlock == nil { + return nil, errors.New("failed to decode CA certificate") + } + caCert, err = x509.ParseCertificate(pemBlock.Bytes) + if err != nil { + return nil, err + } + + caKey := caSecret.Data["tls.key"] + pemBlock, _ = pem.Decode(caKey) + if pemBlock == nil { + return nil, errors.New("failed to decode CA certificate signing key") + } + caPrivateKey, err = x509.ParsePKCS1PrivateKey(pemBlock.Bytes) + if err != nil { + return nil, err + } } serialNumber := big.NewInt(rand2.Int63()) @@ -51,12 +71,22 @@ func generateCertificateSecret(name string, namespace string, caSecret *corev1.S KeyUsage: x509.KeyUsageDigitalSignature, } + if caCert == nil { + // No CA certificate provided so create self-signed certificate + caCert = cert + } + // generate cert private key certPrivateKey, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { return nil, err } + if caPrivateKey == nil { + // No CA certificate provided so create self-signed certificate + caPrivateKey = certPrivateKey + } + privateKeyBytes := x509.MarshalPKCS1PrivateKey(certPrivateKey) // encode for storing into secret privateKeyPem := pem.EncodeToMemory( diff --git a/pkg/controller/hawtio/hawtio_controller.go b/pkg/controller/hawtio/hawtio_controller.go index 81a1390b8f..e73bf0cdbf 100644 --- a/pkg/controller/hawtio/hawtio_controller.go +++ b/pkg/controller/hawtio/hawtio_controller.go @@ -349,7 +349,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque if date := hawtio.Spec.Auth.ClientCertExpirationDate; date != nil && !date.IsZero() { expirationDate = date.Time } - certSecret, err := generateCertificateSecret(clientSecretName, request.Namespace, caSecret, commonName, expirationDate) + certSecret, err := generateCASignedCertSecret(clientSecretName, request.Namespace, caSecret, commonName, expirationDate) if err != nil { reqLogger.Error(err, "Generating the client certificate failed") return reconcile.Result{}, err From 3a2d1729b5a7647f090cce1e25281450f65ec8ae Mon Sep 17 00:00:00 2001 From: phantomjinx Date: Fri, 5 Jan 2024 16:36:31 +0000 Subject: [PATCH 5/8] fix: Adds uninstall rule to Makefile #100 * Removes all of setup, operator and app resources --- Makefile | 55 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 23a1b806ef..afc43aeeb3 100644 --- a/Makefile +++ b/Makefile @@ -53,7 +53,7 @@ ifeq ($(DEBUG),true) GOFLAGS += -gcflags="all=-N -l" endif -.PHONY: image build compile go-generate test manifests k8s-generate install deploy bundle controller-gen kustomize setup operator app +.PHONY: image build compile go-generate test manifests k8s-generate install deploy bundle controller-gen kubectl kustomize check-admin setup operator app # # Function for editing kustomize parameters @@ -135,7 +135,7 @@ get-version: #=== Can only be executed as a cluster-admin # #--- -deploy-crd: +deploy-crd: kubectl kubectl apply -f $(INSTALL_ROOT)/crd/hawt.io_hawtios.yaml #--- @@ -153,7 +153,7 @@ deploy-crd: #** DEBUG: Print the resources to be applied instead of applying them [ true | false ] # #--- -deploy: install kustomize +deploy: kubectl kustomize install $(call set-kvars,$(INSTALL_ROOT)) ifeq ($(DEBUG), false) $(KUSTOMIZE) build $(KOPTIONS) $(INSTALL_ROOT) | kubectl apply -f - @@ -274,6 +274,13 @@ bundle-index: opm yq CSV_SKIPS=$(CSV_SKIP_RANGE) CSV_REPLACES=$(CSV_REPLACES) CHANNELS="$(CHANNELS)" \ ./script/build_bundle_index.sh +# +# Checks if the cluster user has the necessary privileges to be a cluster-admin +# In this case if the user can list the CRDs then probably a cluster-admin +# +check-admin: kubectl + @output=$$(kubectl get crd 2>&1) || (echo "****" && echo "**** ERROR: Cannot continue as user is not a Cluster-Admin ****" && echo "****"; exit 1) + # find or download controller-gen # download controller-gen if necessary controller-gen: @@ -284,6 +291,11 @@ else CONTROLLER_GEN=$(shell command -v controller-gen 2> /dev/null) endif +kubectl: +ifeq (, $(shell command -v kubectl 2> /dev/null)) + $(error "No kubectl found in PATH. Please install and re-run") +endif + kustomize: ifeq (, $(shell command -v kustomize 2> /dev/null)) go install sigs.k8s.io/kustomize/kustomize/v4@$(KUSTOMIZE_VERSION) @@ -349,12 +361,14 @@ endif # #== Setup the installation by installing crds, roles and granting privileges for the installing user. # +#=== Calls check-admin +# #* PARAMETERS: #** IMAGE: Set a custom image for the deployment #** VERSION: Set a custom version for the deployment #** NAMESPACE: Set the namespace for the resources #** DEBUG: Print the resources to be applied instead of applying them [ true | false ] -setup: kustomize +setup: kubectl kustomize check-admin #@ Must be invoked by a user with cluster-admin privileges $(call set-kvars,$(INSTALL_ROOT)/setup) ifeq ($(DEBUG), false) @@ -378,7 +392,7 @@ endif #** DEBUG: Print the resources to be applied instead of applying them [ true | false ] # #--- -operator: kustomize +operator: kubectl kustomize #@ Can be invoked by a user with namespace privileges (rather than a cluster-admin) $(call set-kvars,$(INSTALL_ROOT)/operator) ifeq ($(DEBUG), false) @@ -402,15 +416,40 @@ endif #** DEBUG: Print the resources to be applied instead of applying them [ true | false ] # #--- -app: operator kustomize +app: kubectl kustomize operator #@ Can be invoked by a user with namespace privileges (rather than a cluster-admin) $(call set-kvars,$(INSTALL_ROOT)/app) ifeq ($(DEBUG), false) - $(KUSTOMIZE) build $(KOPTIONS) $(INSTALL_ROOT)/app | kubectl apply -f - + $(KUSTOMIZE) build $(KOPTIONS) $(INSTALL_ROOT)/app | kubectl apply -f - +else + $(KUSTOMIZE) build $(KOPTIONS) $(INSTALL_ROOT)/app +endif + +UNINSTALLS = .uninstall-app .uninstall-operator .uninstall-setup + +$(UNINSTALLS): kubectl kustomize + @$(call set-kvars,$(INSTALL_ROOT)/$(subst .uninstall-,,$@)) +ifeq ($(DEBUG), false) + @$(KUSTOMIZE) build $(KOPTIONS) $(INSTALL_ROOT)/$(subst .uninstall-,,$@) | kubectl delete --ignore-not-found=true -f - else - $(KUSTOMIZE) build $(KOPTIONS) $(INSTALL_ROOT)/app + @$(KUSTOMIZE) build $(KOPTIONS) $(INSTALL_ROOT)/$(subst .uninstall-,,$@) | kubectl delete --dry-run=client -f - endif +#--- +# +#@ uninstall +# +#== Uninstalls the app CR, operator and setup resources +# +#=== Calls check-admin +# +#* PARAMETERS: +#** NAMESPACE: Set the namespace for the resources +#** DEBUG: Print the resources to be deleted instead of deleting them [ true | false ] +# +#--- +uninstall: kubectl kustomize check-admin $(UNINSTALLS) + .DEFAULT_GOAL := help .PHONY: help help: ## Show this help screen. From 217d75c3379f6b82daeea54b8d61d2a842f49eae Mon Sep 17 00:00:00 2001 From: phantomjinx Date: Fri, 5 Jan 2024 18:16:35 +0000 Subject: [PATCH 6/8] Adds rule to Makefile to install only the CR #100 --- Makefile | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Makefile b/Makefile index afc43aeeb3..9259c0536d 100644 --- a/Makefile +++ b/Makefile @@ -401,6 +401,26 @@ else $(KUSTOMIZE) build $(KOPTIONS) $(INSTALL_ROOT)/operator endif +#--- +# +#@ cr +# +#== Install the app CR only +# +#* PARAMETERS: +#** NAMESPACE: Set the namespace for the resources +#** DEBUG: Print the resources to be applied instead of applying them [ true | false ] +# +#--- +cr: kubectl kustomize + #@ Can be invoked by a user with namespace privileges (rather than a cluster-admin) + $(call set-kvars,$(INSTALL_ROOT)/app) +ifeq ($(DEBUG), false) + $(KUSTOMIZE) build $(KOPTIONS) $(INSTALL_ROOT)/app | kubectl apply -f - +else + $(KUSTOMIZE) build $(KOPTIONS) $(INSTALL_ROOT)/app +endif + #--- # #@ app From 5b745738afb763d7659a0c51dacafaeb1cfc7e51 Mon Sep 17 00:00:00 2001 From: phantomjinx Date: Mon, 15 Jan 2024 13:25:57 +0000 Subject: [PATCH 7/8] Implements support for installation on kubernetes #100 * Chooses between route (openshift-only) and ingress API types * Installs self-signed certificate for kubernetes installs for the hawtio-online-serving secret * Only reconcile an OAuth client if on OpenShift for now * Ensures the HAWTIO_ONLINE_AUTH envvar is always populated to specify the authentication type rather than relying on the fallback in the actual hawtio-online image config.sh script * Differentiate between openshift and k8 code by putting in different files and packages if possible --- pkg/controller/hawtio/fakeclient_test.go | 8 +- pkg/controller/hawtio/hawtio_controller.go | 362 +++++++++--------- .../hawtio/hawtio_controller_kubernetes.go | 62 +++ .../hawtio/hawtio_controller_openshift.go | 99 +++++ .../hawtio/hawtio_controller_test.go | 4 + pkg/resources/deployment.go | 16 +- pkg/resources/environment.go | 18 +- pkg/resources/kubernetes/ingress.go | 69 ++++ pkg/resources/label.go | 14 +- pkg/resources/label_test.go | 4 +- pkg/resources/{ => openshift}/route.go | 12 +- pkg/resources/service.go | 8 +- 12 files changed, 461 insertions(+), 215 deletions(-) create mode 100644 pkg/controller/hawtio/hawtio_controller_kubernetes.go create mode 100644 pkg/controller/hawtio/hawtio_controller_openshift.go create mode 100644 pkg/resources/kubernetes/ingress.go rename pkg/resources/{ => openshift}/route.go (87%) diff --git a/pkg/controller/hawtio/fakeclient_test.go b/pkg/controller/hawtio/fakeclient_test.go index 87de5d37c2..dc37598378 100644 --- a/pkg/controller/hawtio/fakeclient_test.go +++ b/pkg/controller/hawtio/fakeclient_test.go @@ -8,15 +8,16 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/version" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" "sigs.k8s.io/controller-runtime/pkg/client/fake" routev1 "github.com/openshift/api/route/v1" - fakeconfig "github.com/openshift/client-go/config/clientset/versioned/fake" fakeoauth "github.com/openshift/client-go/oauth/clientset/versioned/fake" + networkingv1 "k8s.io/api/networking/v1" discoveryfake "k8s.io/client-go/discovery/fake" fakekube "k8s.io/client-go/kubernetes/fake" @@ -43,6 +44,11 @@ func buildReconcileWithFakeClientWithMocks(objs []runtime.Object, t *testing.T) assert.Fail(t, "unable to build scheme") } + err = networkingv1.AddToScheme(scheme) + if err != nil { + assert.Fail(t, "unable to build scheme") + } + err = apis.AddToScheme(scheme) if err != nil { assert.Fail(t, "unable to build scheme") diff --git a/pkg/controller/hawtio/hawtio_controller.go b/pkg/controller/hawtio/hawtio_controller.go index e73bf0cdbf..b0d33237a3 100644 --- a/pkg/controller/hawtio/hawtio_controller.go +++ b/pkg/controller/hawtio/hawtio_controller.go @@ -19,7 +19,7 @@ import ( corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/api/errors" + kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -47,6 +47,8 @@ import ( "github.com/hawtio/hawtio-operator/pkg/capabilities" "github.com/hawtio/hawtio-operator/pkg/openshift" "github.com/hawtio/hawtio-operator/pkg/resources" + kresources "github.com/hawtio/hawtio-operator/pkg/resources/kubernetes" + oresources "github.com/hawtio/hawtio-operator/pkg/resources/openshift" "github.com/hawtio/hawtio-operator/pkg/util" ) @@ -213,9 +215,10 @@ type ReconcileHawtio struct { type DeploymentConfiguration struct { openShiftConsoleURL string configMap *corev1.ConfigMap - clientCertSecret *corev1.Secret - tlsRouteSecret *corev1.Secret - caCertRouteSecret *corev1.Secret + clientCertSecret *corev1.Secret // -proxying certificate secret + tlsRouteSecret *corev1.Secret // custom route certificate secret + caCertRouteSecret *corev1.Secret // custom CA certificate secret + servingCertSecret *corev1.Secret // -serving certificate secret } // Reconcile reads that state of the cluster for a Hawtio object and makes changes based on the state read @@ -231,7 +234,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque hawtio := &hawtiov1.Hawtio{} err := r.client.Get(ctx, request.NamespacedName, hawtio) if err != nil { - if errors.IsNotFound(err) { + if kerrors.IsNotFound(err) { // Request object not found, could have been deleted after reconcile request. // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. // Return and don't requeue @@ -242,8 +245,6 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque } deploymentConfig := DeploymentConfiguration{} - // This secret name should be the same as used in deployment.go - clientSecretName := hawtio.Name + "-tls-proxying" reqLogger.Info(fmt.Sprintf("Cluster API Specification: %+v", r.apiSpec)) @@ -308,7 +309,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque // Retrieve OpenShift Web console public URL cm, err := r.coreClient.ConfigMaps("openshift-config-managed").Get(ctx, "console-public", metav1.GetOptions{}) if err != nil { - if !errors.IsNotFound(err) && !errors.IsForbidden(err) { + if !kerrors.IsNotFound(err) && !kerrors.IsForbidden(err) { reqLogger.Error(err, "Error getting OpenShift managed configuration") return reconcile.Result{}, err } @@ -318,82 +319,40 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque } if r.apiSpec.IsOpenShift4 { - cronJob := &batchv1.CronJob{} - cronJobName := request.Name + "-certificate-expiry-check" - cronJobErr := r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: cronJobName}, cronJob) - - // Check whether client certificate secret exists - clientCertSecret := corev1.Secret{} - err := r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: clientSecretName}, &clientCertSecret) - // TODO: Update the client certificate when the Hawtio resource changes + // Create -proxying certificate + // -serving certificate is automatically created + clientCertSecret, err := osCreateClientCertificate(ctx, r, hawtio, request.Name, request.Namespace) if err != nil { - if errors.IsNotFound(err) { - reqLogger.Info("Client certificate secret not found, creating a new one", "secret", clientSecretName) - - caSecret, err := r.coreClient.Secrets("openshift-service-ca").Get(ctx, "signing-key", metav1.GetOptions{}) - if err != nil { - reqLogger.Error(err, "Reading certificate authority signing key failed") - return reconcile.Result{}, err - } + reqLogger.Error(err, "Failed to create OpenShift proxying certificate") + return reconcile.Result{}, err + } + deploymentConfig.clientCertSecret = clientCertSecret + } else { + // Create -serving certificate + servingCertSecret, err := kubeCreateServingCertificate(ctx, r, hawtio, request.Name, request.Namespace) + if err != nil { + reqLogger.Error(err, "Failed to create serving certificate") + return reconcile.Result{}, err + } + deploymentConfig.servingCertSecret = servingCertSecret + } - commonName := hawtio.Spec.Auth.ClientCertCommonName - if commonName == "" { - if r.ClientCertCommonName == "" { - commonName = "hawtio-online.hawtio.svc" - } else { - commonName = r.ClientCertCommonName - } - } - // Let's default to one year validity period - expirationDate := time.Now().AddDate(1, 0, 0) - if date := hawtio.Spec.Auth.ClientCertExpirationDate; date != nil && !date.IsZero() { - expirationDate = date.Time - } - certSecret, err := generateCASignedCertSecret(clientSecretName, request.Namespace, caSecret, commonName, expirationDate) - if err != nil { - reqLogger.Error(err, "Generating the client certificate failed") - return reconcile.Result{}, err - } - err = controllerutil.SetControllerReference(hawtio, certSecret, r.scheme) - if err != nil { - return reconcile.Result{}, err - } - _, err = r.coreClient.Secrets(request.Namespace).Create(ctx, certSecret, metav1.CreateOptions{}) - reqLogger.Info("Client certificate created successfully", "secret", clientSecretName) - if err != nil { - reqLogger.Error(err, "Creating the client certificate secret failed") - return reconcile.Result{}, err - } + // + // Custom Route certificates defined in Hawtio CR + // + if secretName := hawtio.Spec.Route.CertSecret.Name; secretName != "" { + deploymentConfig.tlsRouteSecret = &corev1.Secret{} + err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: secretName}, deploymentConfig.tlsRouteSecret) + if err != nil { + return reconcile.Result{}, err + } + } - // check if certificate rotation is enabled - if hawtio.Spec.Auth.ClientCertCheckSchedule != "" { - // generate auto-renewal cron job for the secret if it already hasn't been generated. - if cronJobErr != nil && errors.IsNotFound(cronJobErr) { - pod, err := getOperatorPod(ctx, r.client, request.Namespace) - if err != nil { - return reconcile.Result{}, err - } - - //create cronJob to validate the Cert - cronJob = createCertValidationCronJob(cronJobName, request.Namespace, - hawtio.Spec.Auth.ClientCertCheckSchedule, pod.Spec.ServiceAccountName, pod.Spec.Containers[0], - hawtio.Spec.Auth.ClientCertExpirationPeriod) - - err = controllerutil.SetControllerReference(hawtio, cronJob, r.scheme) - if err != nil { - return reconcile.Result{}, err - } - err = r.client.Create(ctx, cronJob) - - if err != nil { - log.Error(err, "Cronjob haven't been created") - return reconcile.Result{}, err - } - } - } - } else { - return reconcile.Result{}, err - } + if caCertSecretName := hawtio.Spec.Route.CaCert.Name; caCertSecretName != "" { + deploymentConfig.caCertRouteSecret = &corev1.Secret{} + err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: caCertSecretName}, deploymentConfig.caCertRouteSecret) + if err != nil { + return reconcile.Result{}, err } } @@ -402,7 +361,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque var rbacConfigMap corev1.ConfigMap err := r.client.Get(ctx, types.NamespacedName{Namespace: request.Namespace, Name: cm}, &rbacConfigMap) if err != nil { - if errors.IsNotFound(err) { + if kerrors.IsNotFound(err) { reqLogger.Info("RBAC ConfigMap must be created", "ConfigMap", cm) // Let's poll for the RBAC ConfigMap to be created return reconcile.Result{Requeue: true, RequeueAfter: 5 * time.Second}, nil @@ -431,52 +390,51 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque return reconcile.Result{}, err } - deploymentConfig.clientCertSecret = &corev1.Secret{} - err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: clientSecretName}, deploymentConfig.clientCertSecret) + _, err = r.reconcileDeployment(hawtio, deploymentConfig) if err != nil { - // clientCertSecret is only used on OpenShift - deploymentConfig.clientCertSecret = nil + reqLogger.Error(err, "Error reconciling deployment") + return reconcile.Result{}, err } - // - // Custom Route certificates defined in Hawtio CR - // - if secretName := hawtio.Spec.Route.CertSecret.Name; secretName != "" { - deploymentConfig.tlsRouteSecret = &corev1.Secret{} - err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: secretName}, deploymentConfig.tlsRouteSecret) - if err != nil { + var ingress *networkingv1.Ingress + var route *routev1.Route + if r.apiSpec.Routes { + route = &routev1.Route{} + err = r.client.Get(ctx, request.NamespacedName, route) + if err != nil && kerrors.IsNotFound(err) { + return reconcile.Result{Requeue: true}, nil + } else if err != nil { + reqLogger.Error(err, "Failed to get route") return reconcile.Result{}, err } - } - if caCertSecretName := hawtio.Spec.Route.CaCert.Name; caCertSecretName != "" { - deploymentConfig.caCertRouteSecret = &corev1.Secret{} - err = r.client.Get(ctx, client.ObjectKey{Namespace: request.Namespace, Name: caCertSecretName}, deploymentConfig.caCertRouteSecret) - if err != nil { + if route == nil { + err := errors.New("Route could not be found") + reqLogger.Error(err, "Route failure") + return reconcile.Result{}, err + } + } else { + ingress = &networkingv1.Ingress{} + err = r.client.Get(ctx, request.NamespacedName, ingress) + if err != nil && kerrors.IsNotFound(err) { + return reconcile.Result{Requeue: true}, nil + } else if err != nil { + reqLogger.Error(err, "Failed to get ingress") return reconcile.Result{}, err } - } - - _, err = r.reconcileDeployment(hawtio, deploymentConfig) - if err != nil { - reqLogger.Error(err, "Error reconciling deployment") - return reconcile.Result{}, err - } - route := &routev1.Route{} - err = r.client.Get(ctx, request.NamespacedName, route) - if err != nil && errors.IsNotFound(err) { - return reconcile.Result{Requeue: true}, nil - } else if err != nil { - reqLogger.Error(err, "Failed to get route") - return reconcile.Result{}, err + if ingress == nil { + err := errors.New("Ingress could not be found") + reqLogger.Error(err, "Ingress failure") + return reconcile.Result{}, err + } } if isClusterDeployment { // Add OAuth client oauthClient := resources.NewOAuthClient(resources.OAuthClientName) err = r.client.Create(ctx, oauthClient) - if err != nil && !errors.IsAlreadyExists(err) { + if err != nil && !kerrors.IsAlreadyExists(err) { return reconcile.Result{}, err } } @@ -490,11 +448,13 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque // Add link to OpenShift console consoleLinkName := request.Name + "-" + request.Namespace - if r.apiSpec.IsOpenShift4 && hawtio.Status.Phase == hawtiov1.HawtioPhaseInitialized { + if r.apiSpec.IsOpenShift4 && r.apiSpec.Routes && hawtio.Status.Phase == hawtiov1.HawtioPhaseInitialized { + // With checks above, route should not be null + consoleLink := &consolev1.ConsoleLink{} err = r.client.Get(ctx, types.NamespacedName{Name: consoleLinkName}, consoleLink) if err != nil { - if !errors.IsNotFound(err) { + if !kerrors.IsNotFound(err) { reqLogger.Error(err, "Failed to get console link") return reconcile.Result{}, err } @@ -505,6 +465,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque return reconcile.Result{}, err } } + consoleLink = &consolev1.ConsoleLink{} if isClusterDeployment { consoleLink = openshift.NewApplicationMenuLink(consoleLinkName, route, hawtconfig) @@ -537,7 +498,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque deployment := &appsv1.Deployment{} err = r.client.Get(ctx, request.NamespacedName, deployment) - if err != nil && errors.IsNotFound(err) { + if err != nil && kerrors.IsNotFound(err) { return reconcile.Result{Requeue: true}, nil } else if err != nil { reqLogger.Error(err, "Failed to get deployment") @@ -557,30 +518,37 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque // Reconcile Hawtio status image field from deployment container image hawtioCopy.Status.Image = deployment.Spec.Template.Spec.Containers[0].Image - // Reconcile route URL into Hawtio status - url := resources.GetRouteURL(route) - hawtioCopy.Status.URL = url + var ingressRouteURL string + if r.apiSpec.Routes { + // With checks above, route should not be null - // Reconcile route host from routeHostName field - if hostName := hawtio.Spec.RouteHostName; len(hostName) == 0 && !strings.EqualFold(route.Annotations[hostGeneratedAnnotation], "true") { - // Emptying route host is ignored so it's not possible to re-generate the host - // See https://github.com/openshift/origin/pull/9425 - // In that case, let's delete the route - err := r.client.Delete(ctx, route) - if err != nil { - reqLogger.Error(err, "Failed to delete route to auto-generate hostname") - return reconcile.Result{}, err + // Reconcile route URL into Hawtio status + ingressRouteURL = oresources.GetRouteURL(route) + hawtioCopy.Status.URL = ingressRouteURL + + // Reconcile route host from routeHostName field + if hostName := hawtio.Spec.RouteHostName; len(hostName) == 0 && !strings.EqualFold(route.Annotations[hostGeneratedAnnotation], "true") { + // Emptying route host is ignored so it's not possible to re-generate the host + // See https://github.com/openshift/origin/pull/9425 + // In that case, let's delete the route + err := r.client.Delete(ctx, route) + if err != nil { + reqLogger.Error(err, "Failed to delete route to auto-generate hostname") + return reconcile.Result{}, err + } + // And requeue to create a new route in the next reconcile loop + return reconcile.Result{Requeue: true}, nil } - // And requeue to create a new route in the next reconcile loop - return reconcile.Result{Requeue: true}, nil } // Reconcile console link in OpenShift console - if r.apiSpec.IsOpenShift4 { + if r.apiSpec.IsOpenShift4 && r.apiSpec.Routes { + // With checks above, route should not be null + consoleLink := &consolev1.ConsoleLink{} err = r.client.Get(ctx, types.NamespacedName{Name: consoleLinkName}, consoleLink) if err != nil { - if errors.IsNotFound(err) { + if kerrors.IsNotFound(err) { // If not found, create a console link if isClusterDeployment { consoleLink = openshift.NewApplicationMenuLink(consoleLinkName, route, hawtconfig) @@ -612,6 +580,7 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque } } } + // Reconcile the client certificate cronJob cronJob := &batchv1.CronJob{} cronJobName := request.Name + "-certificate-expiry-check" @@ -637,55 +606,60 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque } } - // Reconcile OAuth client - // Do not use the default client whose cached informers require - // permission to list cluster wide oauth clients - // err = r.client.Get(ctx, types.NamespacedName{Name: resources.OAuthClientName}, oc) - oc, err := r.oauthClient.OauthV1().OAuthClients().Get(ctx, resources.OAuthClientName, metav1.GetOptions{}) - if err != nil { - if errors.IsNotFound(err) { - // OAuth client should not be found for namespace deployment type - // except when it changes from "cluster" to "namespace" - if isClusterDeployment { - return reconcile.Result{Requeue: true}, nil - } - } else if !(errors.IsForbidden(err) && isNamespaceDeployment) { - // We tolerate 403 for namespace deployment as the operator - // may not have permission to read cluster wide resources - // like OAuth clients - reqLogger.Error(err, "Failed to get OAuth client") - return reconcile.Result{}, err - } - } - // TODO: OAuth client reconciliation triggered by roll-out deployment should ideally - // wait until the deployment is successful before deleting resources - if isClusterDeployment { - // First remove old URL from OAuthClient - if resources.RemoveRedirectURIFromOauthClient(oc, hawtio.Status.URL) { - err := r.client.Update(ctx, oc) - if err != nil { - reqLogger.Error(err, "Failed to reconcile OAuth client") + /* + * Reconcile OAuth client + * Do not use the default client whose cached informers require + * permission to list cluster wide oauth clients + */ + if r.apiSpec.IsOpenShift4 { + oc, err := r.oauthClient.OauthV1().OAuthClients().Get(ctx, resources.OAuthClientName, metav1.GetOptions{}) + if err != nil { + if kerrors.IsNotFound(err) { + // OAuth client should not be found for namespace deployment type + // except when it changes from "cluster" to "namespace" + if isClusterDeployment { + return reconcile.Result{Requeue: true}, nil + } + } else if !(kerrors.IsForbidden(err) && isNamespaceDeployment) { + // We tolerate 403 for namespace deployment as the operator + // may not have permission to read cluster wide resources + // like OAuth clients + reqLogger.Error(err, "Failed to get OAuth client") return reconcile.Result{}, err } } - // Add route URL to OAuthClient authorized redirect URIs - if ok, _ := resources.OauthClientContainsRedirectURI(oc, url); !ok { - oc.RedirectURIs = append(oc.RedirectURIs, url) - err := r.client.Update(ctx, oc) - if err != nil { - reqLogger.Error(err, "Failed to reconcile OAuth client") - return reconcile.Result{}, err + + // TODO: OAuth client reconciliation triggered by roll-out deployment should ideally + // wait until the deployment is successful before deleting resources + if isClusterDeployment && oc != nil { + // First remove old URL from OAuthClient + if resources.RemoveRedirectURIFromOauthClient(oc, hawtio.Status.URL) { + err := r.client.Update(ctx, oc) + if err != nil { + reqLogger.Error(err, "Failed to reconcile OAuth client") + return reconcile.Result{}, err + } + } + // Add route URL to OAuthClient authorized redirect URIs + if ok, _ := resources.OauthClientContainsRedirectURI(oc, ingressRouteURL); !ok { + oc.RedirectURIs = append(oc.RedirectURIs, ingressRouteURL) + err := r.client.Update(ctx, oc) + if err != nil { + reqLogger.Error(err, "Failed to reconcile OAuth client") + return reconcile.Result{}, err + } } } - } - if isNamespaceDeployment && oc != nil { - // Clean-up OAuth client if any. This happens when the deployment type is changed - // from "cluster" to "namespace". - if resources.RemoveRedirectURIFromOauthClient(oc, url) { - err := r.client.Update(ctx, oc) - if err != nil { - reqLogger.Error(err, "Failed to reconcile OAuth client") - return reconcile.Result{}, err + + if isNamespaceDeployment && oc != nil { + // Clean-up OAuth client if any. This happens when the deployment type is changed + // from "cluster" to "namespace". + if resources.RemoveRedirectURIFromOauthClient(oc, ingressRouteURL) { + err := r.client.Update(ctx, oc) + if err != nil { + reqLogger.Error(err, "Failed to reconcile OAuth client") + return reconcile.Result{}, err + } } } } @@ -711,14 +685,32 @@ func (r *ReconcileHawtio) reconcileDeployment(hawtio *hawtiov1.Hawtio, deploymen if deploymentConfig.clientCertSecret != nil { clientCertSecretVersion = deploymentConfig.clientCertSecret.GetResourceVersion() } + + var deployedResources []client.Object + var resourceListTypes []client.ObjectList + deployment, err := resources.NewDeployment(hawtio, r.apiSpec, deploymentConfig.openShiftConsoleURL, deploymentConfig.configMap.GetResourceVersion(), clientCertSecretVersion, r.BuildVariables) if err != nil { return false, err } + deployedResources = append(deployedResources, deployment) + resourceListTypes = append(resourceListTypes, &appsv1.DeploymentList{}) + service := resources.NewService(hawtio) - route := resources.NewRoute(hawtio, deploymentConfig.tlsRouteSecret, deploymentConfig.caCertRouteSecret) + deployedResources = append(deployedResources, service) + resourceListTypes = append(resourceListTypes, &corev1.ServiceList{}) + + if r.apiSpec.Routes { + route := oresources.NewRoute(hawtio, deploymentConfig.tlsRouteSecret, deploymentConfig.caCertRouteSecret) + deployedResources = append(deployedResources, route) + resourceListTypes = append(resourceListTypes, &routev1.RouteList{}) + } else { + ingress := kresources.NewIngress(hawtio, deploymentConfig.servingCertSecret) + deployedResources = append(deployedResources, ingress) + resourceListTypes = append(resourceListTypes, &networkingv1.IngressList{}) + } var serviceAccount *corev1.ServiceAccount if hawtio.Spec.Type == hawtiov1.NamespaceHawtioDeploymentType { @@ -727,17 +719,11 @@ func (r *ReconcileHawtio) reconcileDeployment(hawtio *hawtiov1.Hawtio, deploymen if err != nil { return false, fmt.Errorf("error UpdateResources : %s", err) } + deployedResources = append(deployedResources, serviceAccount) + resourceListTypes = append(resourceListTypes, &corev1.ServiceAccountList{}) } - return r.reconcileResources(hawtio, - []client.Object{deployment, service, route, serviceAccount}, - []client.ObjectList{ - &corev1.ServiceList{}, - &appsv1.DeploymentList{}, - &routev1.RouteList{}, - &corev1.ServiceAccountList{}, - }, - ) + return r.reconcileResources(hawtio, deployedResources, resourceListTypes) } func (r *ReconcileHawtio) reconcileResources(hawtio *hawtiov1.Hawtio, @@ -830,7 +816,7 @@ func (r *ReconcileHawtio) deletion(ctx context.Context, hawtio *hawtiov1.Hawtio) // Remove URI from OAuth client oc := &oauthv1.OAuthClient{} err := r.client.Get(ctx, types.NamespacedName{Name: resources.OAuthClientName}, oc) - if err != nil && !errors.IsNotFound(err) { + if err != nil && !kerrors.IsNotFound(err) { return fmt.Errorf("failed to get OAuth client: %v", err) } updated := resources.RemoveRedirectURIFromOauthClient(oc, hawtio.Status.URL) @@ -849,7 +835,7 @@ func (r *ReconcileHawtio) deletion(ctx context.Context, hawtio *hawtiov1.Hawtio) }, } err := r.client.Delete(ctx, consoleLink) - if err != nil && !errors.IsNotFound(err) && !meta.IsNoMatchError(err) { + if err != nil && !kerrors.IsNotFound(err) && !meta.IsNoMatchError(err) { return fmt.Errorf("failed to delete console link: %v", err) } diff --git a/pkg/controller/hawtio/hawtio_controller_kubernetes.go b/pkg/controller/hawtio/hawtio_controller_kubernetes.go new file mode 100644 index 0000000000..94dda25149 --- /dev/null +++ b/pkg/controller/hawtio/hawtio_controller_kubernetes.go @@ -0,0 +1,62 @@ +package hawtio + +import ( + "context" + "time" + + hawtiov1 "github.com/hawtio/hawtio-operator/pkg/apis/hawtio/v1" + errs "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +var conKLog = logf.Log.WithName("controller_hawtio_kubernetes") + +func kubeCreateServingCertificate(ctx context.Context, r *ReconcileHawtio, hawtio *hawtiov1.Hawtio, name string, namespace string) (*corev1.Secret, error) { + // This secret name should be the same as used in deployment.go + servingSecretName := hawtio.Name + "-tls-serving" + + // Check whether serving certificate secret exists + servingCertSecret, err := r.coreClient.Secrets(namespace).Get(ctx, servingSecretName, metav1.GetOptions{}) + if err == nil { + return servingCertSecret, nil + } + + if kerrors.IsNotFound(err) { + conKLog.Info("Serving certificate secret not found, creating a new one", "secret", servingSecretName) + + commonName := hawtio.Spec.Auth.ClientCertCommonName + if commonName == "" { + if r.ClientCertCommonName == "" { + commonName = "hawtio-online.hawtio.svc" + } else { + commonName = r.ClientCertCommonName + } + } + // Let's default to one year validity period + expirationDate := time.Now().AddDate(1, 0, 0) + if date := hawtio.Spec.Auth.ClientCertExpirationDate; date != nil && !date.IsZero() { + expirationDate = date.Time + } + servingCertSecret, err := generateSelfSignedCertSecret(servingSecretName, namespace, commonName, expirationDate) + if err != nil { + return nil, errs.Wrap(err, "Generating the serving certificate failed") + } + err = controllerutil.SetControllerReference(hawtio, servingCertSecret, r.scheme) + if err != nil { + return nil, err + } + _, err = r.coreClient.Secrets(namespace).Create(ctx, servingCertSecret, metav1.CreateOptions{}) + if err != nil { + return nil, errs.Wrap(err, "Creating the serving certificate secret failed") + } + + conKLog.Info("Serving certificate created successfully", "secret", servingSecretName) + return servingCertSecret, nil + } + + return nil, err +} diff --git a/pkg/controller/hawtio/hawtio_controller_openshift.go b/pkg/controller/hawtio/hawtio_controller_openshift.go new file mode 100644 index 0000000000..f1d4a6beab --- /dev/null +++ b/pkg/controller/hawtio/hawtio_controller_openshift.go @@ -0,0 +1,99 @@ +package hawtio + +import ( + "context" + "time" + + hawtiov1 "github.com/hawtio/hawtio-operator/pkg/apis/hawtio/v1" + errs "github.com/pkg/errors" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +var conOsLog = logf.Log.WithName("controller_hawtio_openshift") + +func osCreateClientCertificate(ctx context.Context, r *ReconcileHawtio, hawtio *hawtiov1.Hawtio, name string, namespace string) (*corev1.Secret, error) { + // This secret name should be the same as used in deployment.go + clientSecretName := hawtio.Name + "-tls-proxying" + + cronJob := &batchv1.CronJob{} + cronJobName := name + "-certificate-expiry-check" + cronJobErr := r.client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: cronJobName}, cronJob) + + // Check whether client certificate secret exists + clientCertSecret, err := r.coreClient.Secrets(namespace).Get(ctx, clientSecretName, metav1.GetOptions{}) + if err == nil { + return clientCertSecret, nil + } + + if kerrors.IsNotFound(err) { + conOsLog.Info("Client certificate secret not found, creating a new one", "secret", clientSecretName) + + caSecret, err := r.coreClient.Secrets("openshift-service-ca").Get(ctx, "signing-key", metav1.GetOptions{}) + if err != nil { + return nil, errs.Wrap(err, "Reading certificate authority signing key failed") + } + + commonName := hawtio.Spec.Auth.ClientCertCommonName + if commonName == "" { + if r.ClientCertCommonName == "" { + commonName = "hawtio-online.hawtio.svc" + } else { + commonName = r.ClientCertCommonName + } + } + // Let's default to one year validity period + expirationDate := time.Now().AddDate(1, 0, 0) + if date := hawtio.Spec.Auth.ClientCertExpirationDate; date != nil && !date.IsZero() { + expirationDate = date.Time + } + clientCertSecret, err := generateCASignedCertSecret(clientSecretName, namespace, caSecret, commonName, expirationDate) + if err != nil { + return nil, errs.Wrap(err, "Generating the client certificate failed") + } + err = controllerutil.SetControllerReference(hawtio, clientCertSecret, r.scheme) + if err != nil { + return nil, err + } + _, err = r.coreClient.Secrets(namespace).Create(ctx, clientCertSecret, metav1.CreateOptions{}) + conOsLog.Info("Client certificate created successfully", "secret", clientSecretName) + if err != nil { + return nil, errs.Wrap(err, "Creating the client certificate secret failed") + } + + // check if certificate rotation is enabled + if hawtio.Spec.Auth.ClientCertCheckSchedule != "" { + // generate auto-renewal cron job for the secret if it already hasn't been generated. + if cronJobErr != nil && kerrors.IsNotFound(cronJobErr) { + pod, err := getOperatorPod(ctx, r.client, namespace) + if err != nil { + return nil, err + } + + //create cronJob to validate the Cert + cronJob = createCertValidationCronJob(cronJobName, namespace, + hawtio.Spec.Auth.ClientCertCheckSchedule, pod.Spec.ServiceAccountName, pod.Spec.Containers[0], + hawtio.Spec.Auth.ClientCertExpirationPeriod) + + err = controllerutil.SetControllerReference(hawtio, cronJob, r.scheme) + if err != nil { + return nil, err + } + err = r.client.Create(ctx, cronJob) + + if err != nil { + return nil, err + } + } + } + + return clientCertSecret, nil + } + + return nil, err +} diff --git a/pkg/controller/hawtio/hawtio_controller_test.go b/pkg/controller/hawtio/hawtio_controller_test.go index 5459268b84..92bf76e32d 100644 --- a/pkg/controller/hawtio/hawtio_controller_test.go +++ b/pkg/controller/hawtio/hawtio_controller_test.go @@ -160,6 +160,10 @@ func TestHawtioController_Reconcile(t *testing.T) { Name: resources.HawtioRbacEnvVar, Value: "", }, + { + Name: resources.HawtioAuthEnvVar, + Value: "form", + }, }) }) }) diff --git a/pkg/resources/deployment.go b/pkg/resources/deployment.go index 54fc816661..c2c12d6a49 100644 --- a/pkg/resources/deployment.go +++ b/pkg/resources/deployment.go @@ -44,10 +44,10 @@ func NewDeployment(hawtio *hawtiov1.Hawtio, apiSpec *capabilities.ApiServerSpec, func newDeployment(hawtio *hawtiov1.Hawtio, replicas *int32, pts corev1.PodTemplateSpec) *appsv1.Deployment { annotations := map[string]string{} - propagateAnnotations(hawtio, annotations) + PropagateAnnotations(hawtio, annotations) - labels := labelsForHawtio(hawtio.Name) - propagateLabels(hawtio, labels) + labels := LabelsForHawtio(hawtio.Name) + PropagateLabels(hawtio, labels) // Deployment replicas field is defaulted to '1', so we have to equal that defaults, otherwise // the comparison algorithm assumes the requested resource is different, which leads to an infinite @@ -64,7 +64,7 @@ func newDeployment(hawtio *hawtiov1.Hawtio, replicas *int32, pts corev1.PodTempl Spec: appsv1.DeploymentSpec{ Replicas: &r, Selector: &metav1.LabelSelector{ - MatchLabels: labelsForHawtio(hawtio.Name), + MatchLabels: LabelsForHawtio(hawtio.Name), }, Template: pts, Strategy: appsv1.DeploymentStrategy{ @@ -84,7 +84,7 @@ func newPodTemplateSpec(hawtio *hawtiov1.Hawtio, apiSpec *capabilities.ApiServer if clientCertSecretVersion != "" { annotations[clientCertSecretVersionAnnotation] = clientCertSecretVersion } - propagateAnnotations(hawtio, annotations) + PropagateAnnotations(hawtio, annotations) volumeMounts, err := newVolumeMounts(apiSpec.IsOpenShift4, hawtioVersion, hawtio.Spec.RBAC.ConfigMap, buildVariables) if err != nil { @@ -95,7 +95,7 @@ func newPodTemplateSpec(hawtio *hawtiov1.Hawtio, apiSpec *capabilities.ApiServer } volumes := newVolumes(hawtio, apiSpec.IsOpenShift4) - labels := labelsForHawtio(hawtio.Name) + labels := LabelsForHawtio(hawtio.Name) additionalLabels, err := labelUtils.ConvertSelectorToLabelsMap(buildVariables.AdditionalLabels) if err != nil { return corev1.PodTemplateSpec{}, err @@ -103,7 +103,7 @@ func newPodTemplateSpec(hawtio *hawtiov1.Hawtio, apiSpec *capabilities.ApiServer for name, value := range additionalLabels { labels[name] = value } - propagateLabels(hawtio, labels) + PropagateLabels(hawtio, labels) pod := corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ @@ -151,7 +151,7 @@ func newVolumes(hawtio *hawtiov1.Hawtio, isOpenShift4 bool) []corev1.Volume { func newEnvVars(hawtio *hawtiov1.Hawtio, apiSpec *capabilities.ApiServerSpec, openShiftConsoleURL string) []corev1.EnvVar { var envVars []corev1.EnvVar - envVarsForHawtio := envVarsForHawtio(hawtio.Spec.Type, hawtio.Name) + envVarsForHawtio := envVarsForHawtio(hawtio.Spec.Type, hawtio.Name, apiSpec.IsOpenShift4) envVars = append(envVars, envVarsForHawtio...) if apiSpec.IsOpenShift4 { diff --git a/pkg/resources/environment.go b/pkg/resources/environment.go index 541e66b94e..77fb7364d2 100644 --- a/pkg/resources/environment.go +++ b/pkg/resources/environment.go @@ -12,6 +12,7 @@ import ( const ( HawtioTypeEnvVar = "HAWTIO_ONLINE_MODE" HawtioNamespaceEnvVar = "HAWTIO_ONLINE_NAMESPACE" + HawtioAuthEnvVar = "HAWTIO_ONLINE_AUTH" HawtioOAuthClientEnvVar = "HAWTIO_OAUTH_CLIENT_ID" HawtioRbacEnvVar = "HAWTIO_ONLINE_RBAC_ACL" HawtioGatewayEnvVar = "HAWTIO_ONLINE_GATEWAY" @@ -21,9 +22,11 @@ const ( NginxClientBodyBufferSize = "NGINX_CLIENT_BODY_BUFFER_SIZE" NginxProxyBuffers = "NGINX_PROXY_BUFFERS" NginxSubrequestOutputBufferSize = "NGINX_SUBREQUEST_OUTPUT_BUFFER_SIZE" + HawtioAuthTypeForm = "form" + HawtioAuthTypeOAuth = "oauth" ) -func envVarsForHawtio(deploymentType hawtiov1.HawtioDeploymentType, name string) []corev1.EnvVar { +func envVarsForHawtio(deploymentType hawtiov1.HawtioDeploymentType, name string, isOpenShift bool) []corev1.EnvVar { oauthClientId := name if deploymentType == hawtiov1.ClusterHawtioDeploymentType { // Pin to a known name for cluster-wide OAuthClient @@ -41,6 +44,19 @@ func envVarsForHawtio(deploymentType hawtiov1.HawtioDeploymentType, name string) }, } + // Ensure that we provide the correct mode of authentication + var authType string + if isOpenShift { + authType = HawtioAuthTypeOAuth + } else { + authType = HawtioAuthTypeForm + } + authTypeEnvVar := corev1.EnvVar{ + Name: HawtioAuthEnvVar, + Value: authType, + } + envVars = append(envVars, authTypeEnvVar) + if deploymentType == hawtiov1.NamespaceHawtioDeploymentType { envVars = append(envVars, corev1.EnvVar{ Name: HawtioNamespaceEnvVar, diff --git a/pkg/resources/kubernetes/ingress.go b/pkg/resources/kubernetes/ingress.go new file mode 100644 index 0000000000..a29b248a4b --- /dev/null +++ b/pkg/resources/kubernetes/ingress.go @@ -0,0 +1,69 @@ +package kubernetes + +import ( + hawtiov1 "github.com/hawtio/hawtio-operator/pkg/apis/hawtio/v1" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/hawtio/hawtio-operator/pkg/resources" +) + +// NewIngress create a new Ingress resource +func NewIngress(hawtio *hawtiov1.Hawtio, servingSecret *corev1.Secret) *networkingv1.Ingress { + name := hawtio.Name + + annotations := map[string]string{} + annotations["nginx.ingress.kubernetes.io/backend-protocol"] = "HTTPS" + annotations["nginx.ingress.kubernetes.io/force-ssl-redirect"] = "true" + annotations["nginx.ingress.kubernetes.io/rewrite-target"] = "/$1" + + resources.PropagateAnnotations(hawtio, annotations) + + labels := map[string]string{ + resources.LabelAppKey: "hawtio", + } + resources.PropagateLabels(hawtio, labels) + + ingressTLS := networkingv1.IngressTLS{} + if servingSecret != nil { + ingressTLS.SecretName = servingSecret.Name + } + + pathPrefix := networkingv1.PathTypePrefix + + ingress := &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: annotations, + Labels: labels, + Name: name, + }, + Spec: networkingv1.IngressSpec{ + TLS: []networkingv1.IngressTLS{ + ingressTLS, + }, + Rules: []networkingv1.IngressRule{ + { + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{{ + Path: "/(.*)", + PathType: &pathPrefix, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: hawtio.Name, + Port: networkingv1.ServiceBackendPort{ + Number: 443, + }, + }, + }, + }}, + }, + }, + }, + }, + }, + } + + return ingress +} diff --git a/pkg/resources/label.go b/pkg/resources/label.go index b2b9d5b980..8f261696f6 100644 --- a/pkg/resources/label.go +++ b/pkg/resources/label.go @@ -6,19 +6,20 @@ import ( ) const ( - labelAppKey = "app" + LabelAppKey = "app" labelResourceKey = "deployment" ) -// Set labels in a map -func labelsForHawtio(name string) map[string]string { +// LabelsForHawtio Set labels in a map +func LabelsForHawtio(name string) map[string]string { return map[string]string{ - labelAppKey: "hawtio", + LabelAppKey: "hawtio", labelResourceKey: name, } } -func propagateAnnotations(hawtio *hawtiov1.Hawtio, annotations map[string]string) { +// PropagateAnnotations propagate annotations from hawtio CR +func PropagateAnnotations(hawtio *hawtiov1.Hawtio, annotations map[string]string) { for k, v := range hawtio.GetAnnotations() { // Only propagate specified annotations if !util.MatchPatterns(hawtio.Spec.MetadataPropagation.Annotations, k) { @@ -31,7 +32,8 @@ func propagateAnnotations(hawtio *hawtiov1.Hawtio, annotations map[string]string } } -func propagateLabels(hawtio *hawtiov1.Hawtio, labels map[string]string) { +// PropagateLabels propagate labels from hawtio CR +func PropagateLabels(hawtio *hawtiov1.Hawtio, labels map[string]string) { for k, v := range hawtio.GetLabels() { // Only propagate specified labels if !util.MatchPatterns(hawtio.Spec.MetadataPropagation.Labels, k) { diff --git a/pkg/resources/label_test.go b/pkg/resources/label_test.go index dd74709da5..8d0b5483e6 100644 --- a/pkg/resources/label_test.go +++ b/pkg/resources/label_test.go @@ -35,7 +35,7 @@ func TestPropagateAnnotations(t *testing.T) { annotations := map[string]string{ "annotation1": "value0", } - propagateAnnotations(hawtio, annotations) + PropagateAnnotations(hawtio, annotations) assert.Len(t, annotations, 5) assert.Equal(t, "value0", annotations["annotation1"]) assert.Equal(t, "value2", annotations["annotation2"]) @@ -69,7 +69,7 @@ func TestPropagateLabels(t *testing.T) { labels := map[string]string{ "label1": "value0", } - propagateLabels(hawtio, labels) + PropagateLabels(hawtio, labels) assert.Len(t, labels, 5) assert.Equal(t, "value0", labels["label1"]) assert.Equal(t, "value2", labels["label2"]) diff --git a/pkg/resources/route.go b/pkg/resources/openshift/route.go similarity index 87% rename from pkg/resources/route.go rename to pkg/resources/openshift/route.go index e852fe5ff4..367aa9358a 100644 --- a/pkg/resources/route.go +++ b/pkg/resources/openshift/route.go @@ -1,4 +1,4 @@ -package resources +package openshift import ( "time" @@ -9,18 +9,20 @@ import ( routev1 "github.com/openshift/api/route/v1" hawtiov1 "github.com/hawtio/hawtio-operator/pkg/apis/hawtio/v1" + + "github.com/hawtio/hawtio-operator/pkg/resources" ) -func NewRoute(hawtio *hawtiov1.Hawtio, routeTLSSecret, caCertRouteSecret *v1.Secret) *routev1.Route { +func NewRoute(hawtio *hawtiov1.Hawtio, routeTLSSecret *v1.Secret, caCertRouteSecret *v1.Secret) *routev1.Route { name := hawtio.Name annotations := map[string]string{} - propagateAnnotations(hawtio, annotations) + resources.PropagateAnnotations(hawtio, annotations) labels := map[string]string{ - labelAppKey: "hawtio", + resources.LabelAppKey: "hawtio", } - propagateLabels(hawtio, labels) + resources.PropagateLabels(hawtio, labels) route := &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/resources/service.go b/pkg/resources/service.go index 1416359a96..162a726572 100644 --- a/pkg/resources/service.go +++ b/pkg/resources/service.go @@ -14,12 +14,12 @@ func NewService(hawtio *hawtiov1.Hawtio) *corev1.Service { annotations := map[string]string{ "service.beta.openshift.io/serving-cert-secret-name": name + "-tls-serving", } - propagateAnnotations(hawtio, annotations) + PropagateAnnotations(hawtio, annotations) labels := map[string]string{ - labelAppKey: "hawtio", + LabelAppKey: "hawtio", } - propagateLabels(hawtio, labels) + PropagateLabels(hawtio, labels) return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -36,7 +36,7 @@ func NewService(hawtio *hawtiov1.Hawtio) *corev1.Service { TargetPort: intstr.FromString(containerPortName), }, }, - Selector: labelsForHawtio(name), + Selector: LabelsForHawtio(name), SessionAffinity: "None", PublishNotReadyAddresses: true, }, From b75b8ce9eaea521f3abe1a7fcdb4cf5cd3d32019 Mon Sep 17 00:00:00 2001 From: phantomjinx Date: Fri, 12 Jan 2024 19:11:25 +0000 Subject: [PATCH 8/8] feat: Adds notional support for extracting the url into the hawtio.Status #100 * While the extraction of the route url is quite straightforward, the use of ingress path rules and no hosts makes the return of a single string quite difficult. This may well need work in the future to improve the value returned --- pkg/controller/hawtio/hawtio_controller.go | 3 + pkg/resources/kubernetes/ingress.go | 71 +++++++++++++++++++++ pkg/resources/kubernetes/ingress_test.go | 73 ++++++++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100644 pkg/resources/kubernetes/ingress_test.go diff --git a/pkg/controller/hawtio/hawtio_controller.go b/pkg/controller/hawtio/hawtio_controller.go index b0d33237a3..d783dc569f 100644 --- a/pkg/controller/hawtio/hawtio_controller.go +++ b/pkg/controller/hawtio/hawtio_controller.go @@ -539,6 +539,9 @@ func (r *ReconcileHawtio) Reconcile(ctx context.Context, request reconcile.Reque // And requeue to create a new route in the next reconcile loop return reconcile.Result{Requeue: true}, nil } + } else { + ingressRouteURL = kresources.GetIngressURL(ingress) + hawtioCopy.Status.URL = ingressRouteURL } // Reconcile console link in OpenShift console diff --git a/pkg/resources/kubernetes/ingress.go b/pkg/resources/kubernetes/ingress.go index a29b248a4b..c5b5913fb7 100644 --- a/pkg/resources/kubernetes/ingress.go +++ b/pkg/resources/kubernetes/ingress.go @@ -1,6 +1,8 @@ package kubernetes import ( + "strconv" + hawtiov1 "github.com/hawtio/hawtio-operator/pkg/apis/hawtio/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" @@ -67,3 +69,72 @@ func NewIngress(hawtio *hawtiov1.Hawtio, servingSecret *corev1.Secret) *networki return ingress } + +// GetIngressURL determines the full URL of the given ingress +func GetIngressURL(ingress *networkingv1.Ingress) string { + var scheme string + if len(ingress.Spec.TLS) > 0 { + scheme = "https" + } else { + scheme = "http" + } + + host, port := getIngressHostAndPort(ingress) + path := getIngressPath(ingress) + + url := scheme + "://" + host + if len(port) > 0 { + url = url + ":" + port + } + + return url + path +} + +func getIngressHostAndPort(ingress *networkingv1.Ingress) (string, string) { + ingressStatuses := ingress.Status.LoadBalancer.Ingress + if len(ingressStatuses) == 0 { + for _, ingressRule := range ingress.Spec.Rules { + if len(ingressRule.Host) > 0 { + return ingressRule.Host, "" + } + } + + return "*", "" // host must be a wildcard + } + + // get host or ip of the first ingress status available + var host string + port := "" + for _, ingressStatus := range ingress.Status.LoadBalancer.Ingress { + if len(ingressStatus.Hostname) > 0 { + host = ingressStatus.Hostname + } else if len(ingressStatus.IP) > 0 { + host = ingressStatus.IP + } + + if len(host) > 0 { + for _, statusPort := range ingressStatus.Ports { + port = strconv.FormatInt(int64(statusPort.Port), 10) + continue // get the first port + } + } + } + + if len(host) == 0 { + return "*", port + } + + return host, port +} + +func getIngressPath(ingress *networkingv1.Ingress) string { + for _, ingressRule := range ingress.Spec.Rules { + if ingressRule.IngressRuleValue.HTTP != nil { + for _, httpPath := range ingressRule.IngressRuleValue.HTTP.Paths { + return httpPath.Path + } + } + } + + return "/" +} diff --git a/pkg/resources/kubernetes/ingress_test.go b/pkg/resources/kubernetes/ingress_test.go new file mode 100644 index 0000000000..c57c490532 --- /dev/null +++ b/pkg/resources/kubernetes/ingress_test.go @@ -0,0 +1,73 @@ +package kubernetes + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/hawtio/hawtio-operator/pkg/resources" + + "github.com/stretchr/testify/assert" +) + +func TestGetIngressURL(t *testing.T) { + annotations := map[string]string{} + annotations["nginx.ingress.kubernetes.io/backend-protocol"] = "HTTPS" + annotations["nginx.ingress.kubernetes.io/force-ssl-redirect"] = "true" + annotations["nginx.ingress.kubernetes.io/rewrite-target"] = "/$1" + + labels := map[string]string{ + resources.LabelAppKey: "hawtio", + } + pathPrefix := networkingv1.PathTypePrefix + name := "hawtio-online" + + ingress := &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: annotations, + Labels: labels, + Name: name, + }, + Spec: networkingv1.IngressSpec{ + TLS: []networkingv1.IngressTLS{ + { + SecretName: "some-tls-secret", + }, + }, + Rules: []networkingv1.IngressRule{ + { + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{{ + Path: "/(.*)", + PathType: &pathPrefix, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: name, + Port: networkingv1.ServiceBackendPort{ + Number: 443, + }, + }, + }, + }}, + }, + }, + }, + }, + }, + Status: networkingv1.IngressStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + IP: "192.168.99.9", + }, + }, + }, + }, + } + + url := GetIngressURL(ingress) + assert.Equal(t, "https://192.168.99.9/(.*)", url) +}