From 937785a2af877e1e62477d8f6680ece1c52a0926 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Wed, 7 Jun 2023 13:43:57 -0400 Subject: [PATCH 01/19] feat(insights): mount Insights token in Cryostat container Signed-off-by: Elliott Baron --- .../controllers/clustercryostat_controller.go | 13 ++- .../resource_definitions.go | 51 +++++++--- internal/controllers/cryostat_controller.go | 2 +- internal/controllers/openshift.go | 69 ++++++++++++++ internal/controllers/reconciler.go | 93 ++++++++++++++++++- internal/main.go | 21 +++-- 6 files changed, 223 insertions(+), 26 deletions(-) diff --git a/internal/controllers/clustercryostat_controller.go b/internal/controllers/clustercryostat_controller.go index 4042e9a9e..d0bb656fb 100644 --- a/internal/controllers/clustercryostat_controller.go +++ b/internal/controllers/clustercryostat_controller.go @@ -89,6 +89,17 @@ func NewClusterCryostatReconciler(config *ReconcilerConfig) *ClusterCryostatReco // +kubebuilder:rbac:groups=console.openshift.io,resources=consolelinks,verbs=get;create;list;update;delete // +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=* +// XXX +// Note: You cannot restrict create or deletecollection requests by their +// resource name. For create, this limitation is because the name of the +// new object may not be known at authorization time. If you restrict list +// or watch by resourceName, clients must include a metadata.name field +// selector in their list or watch request that matches the specified +// resourceName in order to be authorized. For example, kubectl get +// configmaps --field-selector=metadata.name=my-configmap +// +// https://kubernetes.io/docs/reference/access-authn-authz/rbac/#referring-to-resources + // Reconcile processes a ClusterCryostat CR and manages a Cryostat installation accordingly func (r *ClusterCryostatReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { reqLogger := r.Log.WithValues("Request.Name", request.Name) @@ -116,7 +127,7 @@ func (r *ClusterCryostatReconciler) Reconcile(ctx context.Context, request ctrl. // SetupWithManager sets up the controller with the Manager. func (r *ClusterCryostatReconciler) SetupWithManager(mgr ctrl.Manager) error { - return r.delegate.setupWithManager(mgr, &operatorv1beta1.ClusterCryostat{}, r) + return r.delegate.setupWithManager(mgr, r) } func (r *ClusterCryostatReconciler) GetConfig() *ReconcilerConfig { diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index b8ea48463..d8f063c44 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -310,26 +310,40 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima volumes = append(volumes, eventTemplateVolume) } - // Add Auth properties as a volume if specified (on Openshift) - if openshift && cr.Spec.AuthProperties != nil { - authResourceVolume := corev1.Volume{ - Name: "auth-properties-" + cr.Spec.AuthProperties.ConfigMapName, - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: cr.Spec.AuthProperties.ConfigMapName, - }, - Items: []corev1.KeyToPath{ - { - Key: cr.Spec.AuthProperties.Filename, - Path: "OpenShiftAuthManager.properties", - Mode: &readOnlyMode, + if openshift { + // Add Auth properties as a volume if specified (on Openshift) + if cr.Spec.AuthProperties != nil { + authResourceVolume := corev1.Volume{ + Name: "auth-properties-" + cr.Spec.AuthProperties.ConfigMapName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: cr.Spec.AuthProperties.ConfigMapName, + }, + Items: []corev1.KeyToPath{ + { + Key: cr.Spec.AuthProperties.Filename, + Path: "OpenShiftAuthManager.properties", + Mode: &readOnlyMode, + }, }, }, }, + } + volumes = append(volumes, authResourceVolume) + } + + // Add Insights token secret as a volume + insightsVolume := corev1.Volume{ + Name: "insights-token", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: cr.Name + "-insights-token", + Optional: &[]bool{true}[0], + }, }, } - volumes = append(volumes, authResourceVolume) + volumes = append(volumes, insightsVolume) } var podSc *corev1.PodSecurityContext @@ -759,6 +773,13 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag Value: cr.Spec.AuthProperties.ClusterRoleName, }) } + + // Mount Insights token + mounts = append(mounts, corev1.VolumeMount{ + Name: "insights-token", + MountPath: "/var/run/secrets/operator.cryostat.io/insights-token", + ReadOnly: true, + }) } disableBuiltInDiscovery := cr.Spec.TargetDiscoveryOptions != nil && cr.Spec.TargetDiscoveryOptions.BuiltInDiscoveryDisabled diff --git a/internal/controllers/cryostat_controller.go b/internal/controllers/cryostat_controller.go index 8f53decbb..359ae46a8 100644 --- a/internal/controllers/cryostat_controller.go +++ b/internal/controllers/cryostat_controller.go @@ -97,7 +97,7 @@ func (r *CryostatReconciler) Reconcile(ctx context.Context, request ctrl.Request // SetupWithManager sets up the controller with the Manager. func (r *CryostatReconciler) SetupWithManager(mgr ctrl.Manager) error { - return r.delegate.setupWithManager(mgr, &operatorv1beta1.Cryostat{}, r) + return r.delegate.setupWithManager(mgr, r) } func (r *CryostatReconciler) GetConfig() *ReconcilerConfig { diff --git a/internal/controllers/openshift.go b/internal/controllers/openshift.go index c65ae0ec4..dac67fb5e 100644 --- a/internal/controllers/openshift.go +++ b/internal/controllers/openshift.go @@ -38,14 +38,18 @@ package controllers import ( "context" + "encoding/json" + "errors" "fmt" "regexp" + "strings" "github.com/cryostatio/cryostat-operator/internal/controllers/common" "github.com/cryostatio/cryostat-operator/internal/controllers/model" "github.com/go-logr/logr" configv1 "github.com/openshift/api/config/v1" consolev1 "github.com/openshift/api/console/v1" + corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -60,6 +64,10 @@ func (r *Reconciler) reconcileOpenShift(ctx context.Context, cr *model.CryostatI if err != nil { return err } + err = r.reconcilePullSecret(ctx, cr) + if err != nil { + return err + } return r.addCorsAllowedOriginIfNotPresent(ctx, cr) } @@ -204,3 +212,64 @@ func (r *Reconciler) deleteCorsAllowedOrigins(ctx context.Context, cr *model.Cry reqLogger.Info("Removed from APIServer CORS allowed origins") return nil } + +func (r *Reconciler) reconcilePullSecret(ctx context.Context, cr *model.CryostatInstance) error { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: cr.Name + "-insights-token", + Namespace: cr.InstallNamespace, + }, + } + + token, err := r.getTokenFromPullSecret(ctx) + if err != nil { + // TODO warn instead of fail? + return err + } + + return r.createOrUpdateSecret(ctx, secret, cr.Object, func() error { + if secret.StringData == nil { + secret.StringData = map[string]string{} + } + secret.StringData["token"] = *token + return nil + }) +} + +func (r *Reconciler) getTokenFromPullSecret(ctx context.Context) (*string, error) { + // Get the global pull secret + pullSecret := &corev1.Secret{} + err := r.Client.Get(ctx, types.NamespacedName{Namespace: "openshift-config", Name: "pull-secret"}, pullSecret) + if err != nil { + return nil, err + } + + // Look for the .dockerconfigjson key within it + dockerConfigRaw, pres := pullSecret.Data[corev1.DockerConfigJsonKey] + if !pres { + return nil, fmt.Errorf("no %s key present in pull secret", corev1.DockerConfigJsonKey) + } + + // Unmarshal the .dockerconfigjson into a struct + dockerConfig := struct { + Auths map[string]struct { + Auth string `json:"auth"` + } `json:"auths"` + }{} + err = json.Unmarshal(dockerConfigRaw, &dockerConfig) + if err != nil { + return nil, err + } + + // Look for the "cloud.openshift.com" auth + openshiftAuth, pres := dockerConfig.Auths["cloud.openshift.com"] + if !pres { + return nil, errors.New("no \"cloud.openshift.com\" auth within pull secret") + } + + token := strings.TrimSpace(openshiftAuth.Auth) + if strings.Contains(token, "\n") || strings.Contains(token, "\r") { + return nil, fmt.Errorf("invalid cloud.openshift.com token") + } + return &token, nil +} diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 7fdfcf29d..319cf419b 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -64,9 +64,14 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" ) // ReconcilerConfig contains common configuration parameters for @@ -79,6 +84,10 @@ type ReconcilerConfig struct { IsCertManagerInstalled bool EventRecorder record.EventRecorder RESTMapper meta.RESTMapper + ObjectType client.Object + ListType client.ObjectList + IsNamespaced bool + Namespaces []string common.ReconcilerTLS } @@ -289,10 +298,28 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance) return reconcile.Result{}, nil } -func (r *Reconciler) setupWithManager(mgr ctrl.Manager, obj client.Object, - impl reconcile.Reconciler) error { +func (r *Reconciler) setupWithManager(mgr ctrl.Manager, impl reconcile.Reconciler) error { + gvk, err := apiutil.GVKForObject(r.ObjectType, mgr.GetScheme()) + if err != nil { + return err + } + namespaces := namespacesToSet(r.Namespaces) c := ctrl.NewControllerManagedBy(mgr). - For(obj) + For(r.ObjectType). + // TODO remove this once only AllNamespace mode is supported + WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool { + // Restrict watch for CRs to specified namespaces + if object.GetObjectKind().GroupVersionKind().GroupKind() == gvk.GroupKind() { + _, pres := namespaces[object.GetNamespace()] + if !pres { + return false + } + } + return true + })). + Watches(&source.Kind{Type: &corev1.Secret{}}, + handler.EnqueueRequestsFromMapFunc(r.findObjectsForPullSecret), + builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})) // Watch for changes to secondary resources and requeue the owner Cryostat resources := []client.Object{&appsv1.Deployment{}, &corev1.Service{}, &corev1.Secret{}, &corev1.PersistentVolumeClaim{}, @@ -583,3 +610,63 @@ func mergeLabelsAndAnnotations(dest *metav1.ObjectMeta, srcLabels, srcAnnotation dest.Annotations[k] = v } } + +func (r *Reconciler) findObjectsForPullSecret(secret client.Object) []reconcile.Request { + if secret.GetNamespace() != "openshift-config" || secret.GetName() != "pull-secret" { + return nil + } + namespaces := []string{""} + if r.IsNamespaced { + namespaces = r.Namespaces + } + + result := []reconcile.Request{} + for _, namespace := range namespaces { + instanceList, ok := r.ListType.DeepCopyObject().(client.ObjectList) + if !ok { + r.Log.Error(fmt.Errorf("type %T is not an ObjectList", r.ListType), + "could not allocate a list") + return []reconcile.Request{} + } + + listOps := &client.ListOptions{ + Namespace: namespace, + } + err := r.List(context.Background(), instanceList, listOps) + if err != nil { + r.Log.Error(err, "failed to list %T", r.ObjectType, "namespace", namespace) + return []reconcile.Request{} + } + + items, err := meta.ExtractList(instanceList) + if err != nil { + r.Log.Error(err, "could not get items from list") + return []reconcile.Request{} + } + requests := make([]reconcile.Request, len(items)) + for i, item := range items { + obj, err := meta.Accessor(item) + if err != nil { + r.Log.Error(err, "list contains an invalid item") + } + requests[i] = reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: obj.GetName(), + Namespace: obj.GetNamespace(), + }, + } + // XXX + r.Log.Info(fmt.Sprintf("Enqueueing %T %s for pull-secret change", r.ObjectType, types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}.String())) + } + result = append(result, requests...) + } + return result +} + +func namespacesToSet(namespaces []string) map[string]struct{} { + result := make(map[string]struct{}, len(namespaces)) + for _, namespace := range namespaces { + result[namespace] = struct{}{} + } + return result +} diff --git a/internal/main.go b/internal/main.go index 77f851f4c..b00148f51 100644 --- a/internal/main.go +++ b/internal/main.go @@ -40,6 +40,7 @@ import ( "flag" "fmt" "os" + "strings" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. @@ -104,6 +105,7 @@ func main() { setupLog.Error(err, "unable to get WatchNamespace, "+ "the manager will watch and manage resources in all namespaces") } + namespaces := strings.Split(watchNamespace, ",") ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) @@ -111,7 +113,7 @@ func main() { // when used with ClusterCryostat // https://github.com/cryostatio/cryostat-operator/issues/580 disableCache := []client.Object{} - if len(watchNamespace) > 0 { + if len(namespaces) > 0 { disableCache = append(disableCache, &rbacv1.RoleBinding{}) } @@ -125,8 +127,7 @@ func main() { HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "d696d7ab.redhat.com", - Namespace: watchNamespace, - ClientDisableCacheFor: disableCache, + ClientDisableCacheFor: disableCache, // FIXME can probable remove }) if err != nil { setupLog.Error(err, "unable to start manager") @@ -162,12 +163,15 @@ func main() { setupLog.Info("did not find cert-manager installation") } - config := newReconcilerConfig(mgr, "ClusterCryostat", "clustercryostat-controller", openShift, certManager) + config := newReconcilerConfig(mgr, "ClusterCryostat", "clustercryostat-controller", openShift, + certManager, &operatorv1beta1.ClusterCryostat{}, &operatorv1beta1.ClusterCryostatList{}, + false, namespaces) if err = (controllers.NewClusterCryostatReconciler(config)).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterCryostat") os.Exit(1) } - config = newReconcilerConfig(mgr, "Cryostat", "cryostat-controller", openShift, certManager) + config = newReconcilerConfig(mgr, "Cryostat", "cryostat-controller", openShift, certManager, + &operatorv1beta1.Cryostat{}, &operatorv1beta1.CryostatList{}, true, namespaces) if err = (controllers.NewCryostatReconciler(config)).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Cryostat") os.Exit(1) @@ -213,7 +217,8 @@ func isCertManagerInstalled(client discovery.DiscoveryInterface) (bool, error) { } func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName string, openShift bool, - certManager bool) *controllers.ReconcilerConfig { + certManager bool, objType client.Object, listType client.ObjectList, isNamespaced bool, + namespaces []string) *controllers.ReconcilerConfig { return &controllers.ReconcilerConfig{ Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName(logName), @@ -222,6 +227,10 @@ func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName str IsCertManagerInstalled: certManager, EventRecorder: mgr.GetEventRecorderFor(eventRecorderName), RESTMapper: mgr.GetRESTMapper(), + ObjectType: objType, + ListType: listType, + IsNamespaced: isNamespaced, + Namespaces: namespaces, ReconcilerTLS: common.NewReconcilerTLS(&common.ReconcilerTLSConfig{ Client: mgr.GetClient(), }), From 084d877d9d17e99ae255cf7f9a42e5fd17de635e Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Wed, 7 Jun 2023 13:57:15 -0400 Subject: [PATCH 02/19] Use 0440 mode for mounted token --- .../common/resource_definitions/resource_definitions.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index d8f063c44..743cbfb6f 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -338,8 +338,9 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima Name: "insights-token", VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ - SecretName: cr.Name + "-insights-token", - Optional: &[]bool{true}[0], + SecretName: cr.Name + "-insights-token", + Optional: &[]bool{true}[0], + DefaultMode: &readOnlyMode, }, }, } From d4a0485b663d08db087d4323a37739a6f7381357 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Thu, 8 Jun 2023 17:56:22 -0400 Subject: [PATCH 03/19] Envtest based tests for controller watch changes --- .../controllers/clustercryostat_controller.go | 13 +- .../clustercryostat_controller_test.go | 2 +- internal/controllers/common/common_utils.go | 5 +- internal/controllers/cryostat_controller.go | 13 +- .../controllers/cryostat_controller_test.go | 2 +- internal/controllers/openshift.go | 10 +- internal/controllers/rbac.go | 10 +- internal/controllers/reconciler.go | 56 +++- internal/controllers/reconciler_test.go | 292 ++++++++++++++++-- internal/controllers/suite_test.go | 15 +- internal/main.go | 25 +- internal/test/resources.go | 57 +++- 12 files changed, 431 insertions(+), 69 deletions(-) diff --git a/internal/controllers/clustercryostat_controller.go b/internal/controllers/clustercryostat_controller.go index d0bb656fb..91d8a3648 100644 --- a/internal/controllers/clustercryostat_controller.go +++ b/internal/controllers/clustercryostat_controller.go @@ -60,13 +60,16 @@ type ClusterCryostatReconciler struct { *ReconcilerConfig } -func NewClusterCryostatReconciler(config *ReconcilerConfig) *ClusterCryostatReconciler { +func NewClusterCryostatReconciler(config *ReconcilerConfig) (*ClusterCryostatReconciler, error) { + delegate, err := newReconciler(config, &operatorv1beta1.ClusterCryostat{}, + &operatorv1beta1.ClusterCryostatList{}, false) + if err != nil { + return nil, err + } return &ClusterCryostatReconciler{ ReconcilerConfig: config, - delegate: &Reconciler{ - ReconcilerConfig: config, - }, - } + delegate: delegate, + }, nil } // +kubebuilder:rbac:groups="",resources=pods;services;services/finalizers;endpoints;persistentvolumeclaims;events;configmaps;secrets;serviceaccounts,verbs=* diff --git a/internal/controllers/clustercryostat_controller_test.go b/internal/controllers/clustercryostat_controller_test.go index b75ff3111..8dab29a0f 100644 --- a/internal/controllers/clustercryostat_controller_test.go +++ b/internal/controllers/clustercryostat_controller_test.go @@ -139,6 +139,6 @@ func (t *cryostatTestInput) expectTargetNamespaces() { Expect(*cr.TargetNamespaceStatus).To(ConsistOf(t.TargetNamespaces)) } -func newClusterCryostatController(config *controllers.ReconcilerConfig) controllers.CommonReconciler { +func newClusterCryostatController(config *controllers.ReconcilerConfig) (controllers.CommonReconciler, error) { return controllers.NewClusterCryostatReconciler(config) } diff --git a/internal/controllers/common/common_utils.go b/internal/controllers/common/common_utils.go index 13fcc2ae8..eb817f600 100644 --- a/internal/controllers/common/common_utils.go +++ b/internal/controllers/common/common_utils.go @@ -45,6 +45,7 @@ import ( "strings" "time" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" logf "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -84,9 +85,9 @@ func (o *defaultOSUtils) GenPasswd(length int) string { // ClusterUniqueName returns a name for cluster-scoped objects that is // uniquely identified by a namespace and name. -func ClusterUniqueName(kind string, name string, namespace string) string { +func ClusterUniqueName(gvk *schema.GroupVersionKind, name string, namespace string) string { // Use the SHA256 checksum of the namespaced name as a suffix nn := types.NamespacedName{Namespace: namespace, Name: name} suffix := fmt.Sprintf("%x", sha256.Sum256([]byte(nn.String()))) - return strings.ToLower(kind) + "-" + suffix + return strings.ToLower(gvk.Kind) + "-" + suffix } diff --git a/internal/controllers/cryostat_controller.go b/internal/controllers/cryostat_controller.go index 359ae46a8..ef85361d7 100644 --- a/internal/controllers/cryostat_controller.go +++ b/internal/controllers/cryostat_controller.go @@ -57,13 +57,16 @@ type CryostatReconciler struct { *ReconcilerConfig } -func NewCryostatReconciler(config *ReconcilerConfig) *CryostatReconciler { +func NewCryostatReconciler(config *ReconcilerConfig) (*CryostatReconciler, error) { + delegate, err := newReconciler(config, &operatorv1beta1.Cryostat{}, + &operatorv1beta1.CryostatList{}, true) + if err != nil { + return nil, err + } return &CryostatReconciler{ ReconcilerConfig: config, - delegate: &Reconciler{ - ReconcilerConfig: config, - }, - } + delegate: delegate, + }, nil } // +kubebuilder:rbac:groups=operator.cryostat.io,resources=cryostats,verbs=* diff --git a/internal/controllers/cryostat_controller_test.go b/internal/controllers/cryostat_controller_test.go index 479a12156..96582170e 100644 --- a/internal/controllers/cryostat_controller_test.go +++ b/internal/controllers/cryostat_controller_test.go @@ -50,6 +50,6 @@ var _ = Describe("CryostatController", func() { c.commonTests() }) -func newCryostatController(config *controllers.ReconcilerConfig) controllers.CommonReconciler { +func newCryostatController(config *controllers.ReconcilerConfig) (controllers.CommonReconciler, error) { return controllers.NewCryostatReconciler(config) } diff --git a/internal/controllers/openshift.go b/internal/controllers/openshift.go index dac67fb5e..55c58e53d 100644 --- a/internal/controllers/openshift.go +++ b/internal/controllers/openshift.go @@ -76,26 +76,25 @@ func (r *Reconciler) finalizeOpenShift(ctx context.Context, cr *model.CryostatIn return nil } reqLogger := r.Log.WithValues("Request.Namespace", cr.InstallNamespace, "Request.Name", cr.Name) - err := r.deleteConsoleLink(ctx, newConsoleLink(cr), reqLogger) + err := r.deleteConsoleLink(ctx, r.newConsoleLink(cr), reqLogger) if err != nil { return err } return r.deleteCorsAllowedOrigins(ctx, cr) } -func newConsoleLink(cr *model.CryostatInstance) *consolev1.ConsoleLink { +func (r *Reconciler) newConsoleLink(cr *model.CryostatInstance) *consolev1.ConsoleLink { // Cluster scoped, so use a unique name to avoid conflicts return &consolev1.ConsoleLink{ ObjectMeta: metav1.ObjectMeta{ - Name: common.ClusterUniqueName(cr.Object.GetObjectKind().GroupVersionKind().Kind, - cr.Name, cr.InstallNamespace), + Name: common.ClusterUniqueName(r.gvk, cr.Name, cr.InstallNamespace), }, } } func (r *Reconciler) reconcileConsoleLink(ctx context.Context, cr *model.CryostatInstance) error { reqLogger := r.Log.WithValues("Request.Namespace", cr.InstallNamespace, "Request.Name", cr.Name) - link := newConsoleLink(cr) + link := r.newConsoleLink(cr) url := cr.Status.ApplicationURL if len(url) == 0 { @@ -250,6 +249,7 @@ func (r *Reconciler) getTokenFromPullSecret(ctx context.Context) (*string, error return nil, fmt.Errorf("no %s key present in pull secret", corev1.DockerConfigJsonKey) } + fmt.Println("dockerconfig: " + string(dockerConfigRaw)) // Unmarshal the .dockerconfigjson into a struct dockerConfig := struct { Auths map[string]struct { diff --git a/internal/controllers/rbac.go b/internal/controllers/rbac.go index 083e5c402..a27dfae67 100644 --- a/internal/controllers/rbac.go +++ b/internal/controllers/rbac.go @@ -75,7 +75,7 @@ func (r *Reconciler) reconcileRBAC(ctx context.Context, cr *model.CryostatInstan } func (r *Reconciler) finalizeRBAC(ctx context.Context, cr *model.CryostatInstance) error { - return r.deleteClusterRoleBinding(ctx, newClusterRoleBinding(cr)) + return r.deleteClusterRoleBinding(ctx, r.newClusterRoleBinding(cr)) } func newServiceAccount(cr *model.CryostatInstance) *corev1.ServiceAccount { @@ -172,11 +172,11 @@ func (r *Reconciler) reconcileRoleBinding(ctx context.Context, cr *model.Cryosta return nil } -func newClusterRoleBinding(cr *model.CryostatInstance) *rbacv1.ClusterRoleBinding { +func (r *Reconciler) newClusterRoleBinding(cr *model.CryostatInstance) *rbacv1.ClusterRoleBinding { + fmt.Println("CRB KIND: " + r.gvk.Kind) // XXX return &rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ - Name: common.ClusterUniqueName(cr.Object.GetObjectKind().GroupVersionKind().Kind, - cr.Name, cr.InstallNamespace), + Name: common.ClusterUniqueName(r.gvk, cr.Name, cr.InstallNamespace), }, } } @@ -184,7 +184,7 @@ func newClusterRoleBinding(cr *model.CryostatInstance) *rbacv1.ClusterRoleBindin const clusterRoleName = "cryostat-operator-cryostat" func (r *Reconciler) reconcileClusterRoleBinding(ctx context.Context, cr *model.CryostatInstance) error { - binding := newClusterRoleBinding(cr) + binding := r.newClusterRoleBinding(cr) sa := newServiceAccount(cr) subjects := []rbacv1.Subject{ diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 319cf419b..3e8c8ff11 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -61,6 +61,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -84,9 +85,6 @@ type ReconcilerConfig struct { IsCertManagerInstalled bool EventRecorder record.EventRecorder RESTMapper meta.RESTMapper - ObjectType client.Object - ListType client.ObjectList - IsNamespaced bool Namespaces []string common.ReconcilerTLS } @@ -95,11 +93,16 @@ type ReconcilerConfig struct { // between the ClusterCryostat and Cryostat reconcilers type CommonReconciler interface { reconcile.Reconciler + SetupWithManager(mgr ctrl.Manager) error GetConfig() *ReconcilerConfig } type Reconciler struct { *ReconcilerConfig + objectType client.Object + listType client.ObjectList + isNamespaced bool + gvk *schema.GroupVersionKind } // Name used for Finalizer that handles Cryostat deletion @@ -146,6 +149,21 @@ var reportsDeploymentConditions = deploymentConditionTypeMap{ operatorv1beta1.ConditionTypeReportsDeploymentReplicaFailure: appsv1.DeploymentReplicaFailure, } +func newReconciler(config *ReconcilerConfig, objType client.Object, listType client.ObjectList, + isNamespaced bool) (*Reconciler, error) { + gvk, err := apiutil.GVKForObject(objType, config.Scheme) + if err != nil { + return nil, err + } + return &Reconciler{ + ReconcilerConfig: config, + objectType: objType, + listType: listType, + isNamespaced: isNamespaced, + gvk: &gvk, + }, nil +} + func (r *Reconciler) reconcileCryostat(ctx context.Context, cr *model.CryostatInstance) (ctrl.Result, error) { result, err := r.reconcile(ctx, cr) return result, r.checkConflicts(cr, err) @@ -299,17 +317,24 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance) } func (r *Reconciler) setupWithManager(mgr ctrl.Manager, impl reconcile.Reconciler) error { - gvk, err := apiutil.GVKForObject(r.ObjectType, mgr.GetScheme()) - if err != nil { - return err - } namespaces := namespacesToSet(r.Namespaces) c := ctrl.NewControllerManagedBy(mgr). - For(r.ObjectType). + For(r.objectType). // TODO remove this once only AllNamespace mode is supported WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool { - // Restrict watch for CRs to specified namespaces - if object.GetObjectKind().GroupVersionKind().GroupKind() == gvk.GroupKind() { + // Restrict watch for namespaced CRs to specified namespaces + fmt.Println("KIND: " + object.GetObjectKind().GroupVersionKind().GroupKind().String()) // XXX + gk := object.GetObjectKind().GroupVersionKind().GroupKind() + if gk.Empty() { + gvk, err := apiutil.GVKForObject(object, mgr.GetScheme()) + if err != nil { + r.Log.Info("failed to find GVK for %T %s/%s", object, object.GetNamespace(), object.GetName()) + return false + } + gk = gvk.GroupKind() + fmt.Println("KIND after lookup: " + gvk.GroupKind().String()) // XXX + } + if gk == r.gvk.GroupKind() && len(object.GetNamespace()) > 0 { _, pres := namespaces[object.GetNamespace()] if !pres { return false @@ -613,18 +638,19 @@ func mergeLabelsAndAnnotations(dest *metav1.ObjectMeta, srcLabels, srcAnnotation func (r *Reconciler) findObjectsForPullSecret(secret client.Object) []reconcile.Request { if secret.GetNamespace() != "openshift-config" || secret.GetName() != "pull-secret" { + fmt.Printf("GOT SECRET %s/%s\n", secret.GetNamespace(), secret.GetName()) return nil } namespaces := []string{""} - if r.IsNamespaced { + if r.isNamespaced { namespaces = r.Namespaces } result := []reconcile.Request{} for _, namespace := range namespaces { - instanceList, ok := r.ListType.DeepCopyObject().(client.ObjectList) + instanceList, ok := r.listType.DeepCopyObject().(client.ObjectList) if !ok { - r.Log.Error(fmt.Errorf("type %T is not an ObjectList", r.ListType), + r.Log.Error(fmt.Errorf("type %T is not an ObjectList", r.listType), "could not allocate a list") return []reconcile.Request{} } @@ -634,7 +660,7 @@ func (r *Reconciler) findObjectsForPullSecret(secret client.Object) []reconcile. } err := r.List(context.Background(), instanceList, listOps) if err != nil { - r.Log.Error(err, "failed to list %T", r.ObjectType, "namespace", namespace) + r.Log.Error(err, "failed to list %T", r.objectType, "namespace", namespace) return []reconcile.Request{} } @@ -656,7 +682,7 @@ func (r *Reconciler) findObjectsForPullSecret(secret client.Object) []reconcile. }, } // XXX - r.Log.Info(fmt.Sprintf("Enqueueing %T %s for pull-secret change", r.ObjectType, types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}.String())) + r.Log.Info(fmt.Sprintf("Enqueueing %T %s for pull-secret change", r.objectType, types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}.String())) } result = append(result, requests...) } diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index be05e7c10..d1ff3ffeb 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -39,6 +39,7 @@ package controllers_test import ( "context" "fmt" + "strconv" "time" certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" @@ -59,7 +60,9 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -75,12 +78,13 @@ import ( type controllerTest struct { clusterScoped bool - constructorFunc func(*controllers.ReconcilerConfig) controllers.CommonReconciler + constructorFunc func(*controllers.ReconcilerConfig) (controllers.CommonReconciler, error) } type cryostatTestInput struct { - controller controllers.CommonReconciler - objs []ctrlclient.Object + controller controllers.CommonReconciler + objs []ctrlclient.Object + watchNamespaces []string test.TestReconcilerConfig *test.TestResources } @@ -102,6 +106,7 @@ func (c *controllerTest) commonBeforeEach() *cryostatTestInput { t.objs = []ctrlclient.Object{ t.NewNamespace(), t.NewApiServer(), + t.NewGlobalPullSecret(), // TODO OpenShift only? } return t } @@ -114,11 +119,13 @@ func (c *controllerTest) commonJustBeforeEach(t *cryostatTestInput) { err := test.SetCreationTimestamp(t.objs...) Expect(err).ToNot(HaveOccurred()) t.Client = fake.NewClientBuilder().WithScheme(s).WithObjects(t.objs...).Build() - t.controller = c.constructorFunc(t.newReconcilerConfig(s, t.Client)) + t.controller, err = c.constructorFunc(t.newReconcilerConfig(s, t.Client)) + Expect(err).ToNot(HaveOccurred()) } func (c *controllerTest) commonJustAfterEach(t *cryostatTestInput) { for _, obj := range t.objs { + fmt.Printf("Deleting: %v\n", obj) // XXX err := ctrlclient.IgnoreNotFound(t.Client.Delete(context.Background(), obj)) Expect(err).ToNot(HaveOccurred()) } @@ -136,6 +143,7 @@ func (t *cryostatTestInput) newReconcilerConfig(scheme *runtime.Scheme, client c RESTMapper: test.NewTESTRESTMapper(), Log: logger, ReconcilerTLS: test.NewTestReconcilerTLS(&t.TestReconcilerConfig), + Namespaces: t.watchNamespaces, } } @@ -225,22 +233,26 @@ func expectSuccessful(t **cryostatTestInput) { } func (c *controllerTest) commonTests() { - var t *cryostatTestInput + c.commonTestsWithoutManager() + c.commonTestsWithManager() +} - BeforeEach(func() { - t = c.commonBeforeEach() - t.TargetNamespaces = []string{t.Namespace} - }) +func (c *controllerTest) commonTestsWithoutManager() { + var t *cryostatTestInput - JustBeforeEach(func() { - c.commonJustBeforeEach(t) - }) + Describe("reconciling a request in OpenShift", func() { + BeforeEach(func() { + t = c.commonBeforeEach() + t.TargetNamespaces = []string{t.Namespace} + }) - JustAfterEach(func() { - c.commonJustAfterEach(t) - }) + JustBeforeEach(func() { + c.commonJustBeforeEach(t) + }) - Describe("reconciling a request in OpenShift", func() { + JustAfterEach(func() { + c.commonJustAfterEach(t) + }) Context("with a default CR", func() { BeforeEach(func() { t.objs = append(t.objs, t.NewCryostat().Object) @@ -1285,6 +1297,9 @@ func (c *controllerTest) commonTests() { It("should add application url to APIServer AdditionalCORSAllowedOrigins", func() { t.expectAPIServer() }) + It("should create an Insights token secret", func() { + t.expectInsightsTokenSecret() + }) It("should add the finalizer", func() { t.expectCryostatFinalizerPresent() }) @@ -1292,6 +1307,7 @@ func (c *controllerTest) commonTests() { BeforeEach(func() { t.objs = []ctrlclient.Object{ t.NewCryostat().Object, t.NewNamespaceWithSCCSupGroups(), t.NewApiServer(), + t.NewGlobalPullSecret(), } }) It("should set fsGroup to value derived from namespace", func() { @@ -1763,10 +1779,14 @@ func (c *controllerTest) commonTests() { JustBeforeEach(func() { other.commonJustBeforeEach(otherInput) + // Controllers need to share client to have shared view of objects otherInput.Client = t.Client config := otherInput.newReconcilerConfig(otherInput.Client.Scheme(), otherInput.Client) - otherInput.controller = other.constructorFunc(config) + controller, err := other.constructorFunc(config) + Expect(err).ToNot(HaveOccurred()) + otherInput.controller = controller + // Reconcile conflicting namespaced Cryostat fully otherInput.reconcileCryostatFully() // Try reconciling ClusterCryostat @@ -1858,8 +1878,18 @@ func (c *controllerTest) commonTests() { Describe("reconciling a request in Kubernetes", func() { BeforeEach(func() { + t = c.commonBeforeEach() + t.TargetNamespaces = []string{t.Namespace} t.OpenShift = false }) + + JustBeforeEach(func() { + c.commonJustBeforeEach(t) + }) + + JustAfterEach(func() { + c.commonJustAfterEach(t) + }) Context("with TLS ingress", func() { BeforeEach(func() { t.objs = append(t.objs, t.NewCryostatWithIngress().Object) @@ -2161,6 +2191,210 @@ func (c *controllerTest) commonTests() { }) } +func (c *controllerTest) commonTestsWithManager() { + var t *cryostatTestInput + + Describe("setting up the controller", func() { + var done chan interface{} + var cancel context.CancelFunc + + BeforeEach(func() { + t = &cryostatTestInput{ + TestReconcilerConfig: test.TestReconcilerConfig{ + GeneratedPasswords: []string{"grafana", "credentials_database", "jmx", "keystore"}, + }, + TestResources: &test.TestResources{ + Name: "cryostat", + Namespace: "test", + TLS: true, + ExternalTLS: true, + OpenShift: true, + ClusterScoped: c.clusterScoped, + }, + } + suffix := strconv.Itoa(nameSuffix) + t.Name += "-" + suffix + t.Namespace += "-" + suffix + t.TargetNamespaces = []string{t.Namespace} + t.watchNamespaces = []string{t.Namespace} + + t.TLS = false // TODO can use InstallCertManager + t.objs = []ctrlclient.Object{ + t.NewNamespace(), + t.NewApiServer(), + t.NewGlobalPullSecret(), + } + }) + JustBeforeEach(func() { + k8sClient, err := ctrlclient.New(cfg, ctrlclient.Options{Scheme: scheme.Scheme}) + Expect(err).NotTo(HaveOccurred()) + Expect(k8sClient).NotTo(BeNil()) + + // Create openshift-config namespace first + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "openshift-config", + }, + } + err = k8sClient.Create(context.Background(), ns) + Expect(ctrlclient.IgnoreAlreadyExists(err)).ToNot(HaveOccurred()) + + // Create namespaces first + // nsGeneratedNameMap := map[string]string{} + // otherObjs := []ctrlclient.Object{} + for _, obj := range t.objs { + // if ns, ok := obj.(*corev1.Namespace); ok { + // saveName := ns.Name + // ns.GenerateName = ns.Name + "-" + // ns.Name = "" + // fmt.Printf("Creating: %v\n", ns) // XXX + // err := k8sClient.Create(context.TODO(), obj) + // Expect(err).ToNot(HaveOccurred()) + // fmt.Println("Namespace: " + ns.Name) // XXX + // nsGeneratedNameMap[saveName] = ns.Name + // } else { + // otherObjs = append(otherObjs, obj) + // } + // } + // // Create other objects while substituting namespace + // for _, obj := range otherObjs { + // if len(obj.GetNamespace()) > 0 { + // Expect(nsGeneratedNameMap).To(HaveKey(obj.GetNamespace())) + // ns := nsGeneratedNameMap[obj.GetNamespace()] + // obj.SetNamespace(ns) + // } + fmt.Printf("Creating: %v\n", obj) // XXX + err := k8sClient.Create(context.TODO(), obj) + Expect(err).ToNot(HaveOccurred()) + } + // // Replace namespace variables with generated names + // Expect(nsGeneratedNameMap).To(HaveKey(t.Namespace)) + // t.Namespace = nsGeneratedNameMap[t.Namespace] + // Expect(nsGeneratedNameMap).To(HaveKey(otherNamespace)) + // otherNamespace = nsGeneratedNameMap[otherNamespace] + // t.TargetNamespaces = []string{t.Namespace} + // t.watchNamespaces = []string{t.Namespace} + + t.Client = k8sClient + controller, err := c.constructorFunc(t.newReconcilerConfig(scheme.Scheme, t.Client)) + Expect(err).ToNot(HaveOccurred()) + t.controller = controller + manager, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: test.NewTestScheme(), + MetricsBindAddress: "0", + Port: 9443, + HealthProbeBindAddress: "0", + LeaderElection: false, + }) + Expect(err).ToNot(HaveOccurred()) + err = t.controller.SetupWithManager(manager) + Expect(err).ToNot(HaveOccurred()) + + ctx, fn := context.WithCancel(context.Background()) + cancel = fn + done = make(chan interface{}) + go func() { + defer GinkgoRecover() + err := manager.Start(ctx) + Expect(err).ToNot(HaveOccurred()) + fmt.Println("Stopping Manager") // XXX + close(done) + }() + }) + JustAfterEach(func() { + for _, obj := range t.objs { + fmt.Printf("Deleting: %v\n", obj) // XXX + err := ctrlclient.IgnoreNotFound(t.Client.Delete(context.Background(), obj)) + Expect(err).ToNot(HaveOccurred()) + } + }) + AfterEach(func() { + if cancel != nil { + cancel() + <-done + } + nameSuffix++ + }) + + Context("watches the configured namespace(s)", func() { + var otherNamespace string + + BeforeEach(func() { + suffix := strconv.Itoa(nameSuffix) + saveNS := t.Namespace + otherNamespace = "other-" + suffix + t.Namespace = otherNamespace + otherNS := t.NewNamespace() + t.Namespace = saveNS + t.objs = append(t.objs, otherNS) + }) + Context("creating a CR in the watched namespace", func() { + BeforeEach(func() { + t.objs = append(t.objs, t.NewCryostatWithIngressCertManagerDisabled().Object) + }) + It("should reconcile the CR", func() { + Eventually(func() bool { + cr := t.getCryostatInstance() + if cr != nil && cr.Status != nil && len(cr.Status.ApplicationURL) > 0 { + return true + } + return false + }).WithTimeout(time.Minute).WithPolling(time.Millisecond).Should(BeTrue()) + }) + }) + if !c.clusterScoped { + Context("creating a CR in a different namespace", func() { + BeforeEach(func() { + t.Namespace = otherNamespace + t.TargetNamespaces = []string{otherNamespace} + t.objs = append(t.objs, t.NewCryostatWithIngressCertManagerDisabled().Object) + }) + It("should not reconcile the CR", func() { + Consistently(func() bool { + cr := t.getCryostatInstance() + if cr != nil && cr.Status != nil && len(cr.Status.ApplicationURL) > 0 { + return true + } + return false + }).WithTimeout(time.Minute).WithPolling(time.Millisecond).Should(BeFalse()) + }) + }) + } + }) + + Context("watches the global pull secret", func() { + BeforeEach(func() { + cr := t.NewCryostatWithIngressCertManagerDisabled() + t.objs = append(t.objs, cr.Object) + }) + JustBeforeEach(func() { + // Wait for Insights Token to exist to ensure it will be updated later + Eventually(t.getInsightsTokenValue()).WithTimeout(time.Minute).WithPolling(time.Millisecond).Should(Equal("world")) + // Wait for reconciler to finish + time.Sleep(30 * time.Second) // TODO is there some way to check reconciler queue size? + }) + Context("when the pull secret is updated", func() { + JustBeforeEach(func() { + secret := t.NewGlobalPullSecret() + err := t.Client.Get(context.Background(), types.NamespacedName{Namespace: secret.Namespace, Name: secret.Name}, secret) + Expect(err).ToNot(HaveOccurred()) + + if secret.StringData == nil { + secret.StringData = map[string]string{} + } + secret.StringData[corev1.DockerConfigJsonKey] = `{"auths":{"example.com":{"auth":"hello"},"cloud.openshift.com":{"auth":"foo"}}}` + + err = t.Client.Update(context.Background(), secret) + Expect(err).ToNot(HaveOccurred()) + }) + It("should update the Insights token", func() { + Eventually(t.getInsightsTokenValue()).WithTimeout(time.Minute).WithPolling(time.Millisecond).Should(Equal("foo")) + }) + }) + }) + }) +} + func (t *cryostatTestInput) expectRoutes() { if !t.Minimal { t.checkRoute(t.NewGrafanaRoute()) @@ -2448,6 +2682,16 @@ func (t *cryostatTestInput) expectJMXSecret() { Expect(secret.StringData).To(Equal(expectedSecret.StringData)) } +func (t *cryostatTestInput) expectInsightsTokenSecret() { + expectedSecret := t.NewInsightsTokenSecret() + secret := &corev1.Secret{} + err := t.Client.Get(context.Background(), types.NamespacedName{Name: expectedSecret.Name, Namespace: expectedSecret.Namespace}, secret) + Expect(err).ToNot(HaveOccurred()) + + t.checkMetadata(secret, expectedSecret) + Expect(secret.StringData).To(Equal(expectedSecret.StringData)) +} + func (t *cryostatTestInput) expectCoreService() { t.checkService(t.Name, t.NewCryostatService()) } @@ -2979,7 +3223,19 @@ func (t *cryostatTestInput) expectResourcesUnaffected() { } } -func getControllerFunc(clusterScoped bool) func(*controllers.ReconcilerConfig) controllers.CommonReconciler { +func (t *cryostatTestInput) getInsightsTokenValue() func() string { + tokenSecret := t.NewInsightsTokenSecret() + return func() string { + err := t.Client.Get(context.Background(), types.NamespacedName{Namespace: tokenSecret.Namespace, Name: tokenSecret.Name}, tokenSecret) + Expect(ctrlclient.IgnoreNotFound(err)).ToNot(HaveOccurred()) + if tokenSecret != nil { + return string(tokenSecret.Data["token"]) + } + return "" + } +} + +func getControllerFunc(clusterScoped bool) func(*controllers.ReconcilerConfig) (controllers.CommonReconciler, error) { if clusterScoped { return newClusterCryostatController } diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 8ec8620eb..3cb81c12b 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -37,6 +37,8 @@ package controllers_test import ( + "fmt" + "go/build" "path/filepath" "testing" @@ -64,6 +66,7 @@ import ( var cfg *rest.Config var k8sClient client.Client var testEnv *envtest.Environment +var nameSuffix int func TestAPIs(t *testing.T) { RegisterFailHandler(Fail) @@ -75,9 +78,18 @@ var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) By("bootstrapping test environment") + // TODO hardcoded version + openshiftPrefix := []string{build.Default.GOPATH, "pkg", "mod", "github.com", "openshift", "api@v0.0.0-20230406152840-ce21e3fe5da2"} testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")}, + CRDDirectoryPaths: []string{ + filepath.Join("..", "..", "config", "crd", "bases"), + filepath.Join(append(openshiftPrefix, "route", "v1")...), + filepath.Join(append(openshiftPrefix, "console", "v1")...), + filepath.Join(append(openshiftPrefix, "config", "v1")...), + }, + ErrorIfCRDPathMissing: true, } + fmt.Println(testEnv.CRDDirectoryPaths) var err error // cfg is defined in this file globally. @@ -106,6 +118,7 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient).NotTo(BeNil()) + nameSuffix = 0 }) var _ = AfterSuite(func() { diff --git a/internal/main.go b/internal/main.go index b00148f51..20ada7181 100644 --- a/internal/main.go +++ b/internal/main.go @@ -164,18 +164,27 @@ func main() { } config := newReconcilerConfig(mgr, "ClusterCryostat", "clustercryostat-controller", openShift, - certManager, &operatorv1beta1.ClusterCryostat{}, &operatorv1beta1.ClusterCryostatList{}, - false, namespaces) - if err = (controllers.NewClusterCryostatReconciler(config)).SetupWithManager(mgr); err != nil { + certManager, namespaces) + clusterController, err := controllers.NewClusterCryostatReconciler(config) + if err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterCryostat") os.Exit(1) } + if err = clusterController.SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to add controller to manager", "controller", "ClusterCryostat") + os.Exit(1) + } config = newReconcilerConfig(mgr, "Cryostat", "cryostat-controller", openShift, certManager, - &operatorv1beta1.Cryostat{}, &operatorv1beta1.CryostatList{}, true, namespaces) - if err = (controllers.NewCryostatReconciler(config)).SetupWithManager(mgr); err != nil { + namespaces) + controller, err := controllers.NewCryostatReconciler(config) + if err != nil { setupLog.Error(err, "unable to create controller", "controller", "Cryostat") os.Exit(1) } + if err = controller.SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to add controller to manager", "controller", "Cryostat") + os.Exit(1) + } // +kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { @@ -217,8 +226,7 @@ func isCertManagerInstalled(client discovery.DiscoveryInterface) (bool, error) { } func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName string, openShift bool, - certManager bool, objType client.Object, listType client.ObjectList, isNamespaced bool, - namespaces []string) *controllers.ReconcilerConfig { + certManager bool, namespaces []string) *controllers.ReconcilerConfig { return &controllers.ReconcilerConfig{ Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName(logName), @@ -227,9 +235,6 @@ func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName str IsCertManagerInstalled: certManager, EventRecorder: mgr.GetEventRecorderFor(eventRecorderName), RESTMapper: mgr.GetRESTMapper(), - ObjectType: objType, - ListType: listType, - IsNamespaced: isNamespaced, Namespaces: namespaces, ReconcilerTLS: common.NewReconcilerTLS(&common.ReconcilerTLSConfig{ Client: mgr.GetClient(), diff --git a/internal/test/resources.go b/internal/test/resources.go index 89603bde9..ad8ada49f 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -206,7 +206,14 @@ func (r *TestResources) NewCryostatWithTemplates() *model.CryostatInstance { } func (r *TestResources) NewCryostatWithIngress() *model.CryostatInstance { - cr := r.NewCryostat() + return r.addIngressToCryostat(r.NewCryostat()) +} + +func (r *TestResources) NewCryostatWithIngressCertManagerDisabled() *model.CryostatInstance { + return r.addIngressToCryostat(r.NewCryostatCertManagerDisabled()) +} + +func (r *TestResources) addIngressToCryostat(cr *model.CryostatInstance) *model.CryostatInstance { networkConfig := r.newNetworkConfigurationList() cr.Spec.NetworkOptions = &networkConfig return cr @@ -1625,6 +1632,14 @@ func (r *TestResources) NewCoreVolumeMounts() []corev1.VolumeMount { MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-tls", r.Name), }) } + if r.OpenShift { + mounts = append(mounts, + corev1.VolumeMount{ + Name: "insights-token", + ReadOnly: true, + MountPath: "/var/run/secrets/operator.cryostat.io/insights-token", + }) + } return mounts } @@ -1988,6 +2003,20 @@ func (r *TestResources) newVolumes(certProjections []corev1.VolumeProjection) [] }, }) + if r.OpenShift { + volumes = append(volumes, + corev1.Volume{ + Name: "insights-token", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: r.Name + "-insights-token", + Optional: &[]bool{true}[0], + DefaultMode: &readOnlymode, + }, + }, + }) + } + return volumes } @@ -2769,6 +2798,32 @@ func (r *TestResources) NewLockConfigMap() *corev1.ConfigMap { } } +func (r *TestResources) NewGlobalPullSecret() *corev1.Secret { + config := `{"auths":{"example.com":{"auth":"hello"},"cloud.openshift.com":{"auth":"world"}}}` + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pull-secret", + Namespace: "openshift-config", + }, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(config), + }, + } +} + +func (r *TestResources) NewInsightsTokenSecret() *corev1.Secret { + config := "world" + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: r.Name + "-insights-token", + Namespace: r.Namespace, + }, + StringData: map[string]string{ + "token": config, + }, + } +} + func (r *TestResources) getClusterUniqueName() string { prefix := "cryostat-" if r.ClusterScoped { From 5550af1c4148d980c62038b5328a4fd9351d1258 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Fri, 9 Jun 2023 17:31:14 -0400 Subject: [PATCH 04/19] Fix hardcoded OpenShift module version --- Makefile | 4 +++- internal/controllers/suite_test.go | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index ac79b2786..21b2542cc 100644 --- a/Makefile +++ b/Makefile @@ -124,7 +124,9 @@ test: test-envtest test-scorecard .PHONY: test-envtest test-envtest: generate manifests fmt vet setup-envtest ifneq ($(SKIP_TESTS), true) - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" $(GO_TEST) -v -coverprofile cover.out ./... + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" \ + OPENSHIFT_API_MOD_VERSION=$(shell go list -m -f '{{if .Replace}}{{.Replace.Version}}{{else}}{{.Version}}{{end}}' github.com/openshift/api) \ + $(GO_TEST) -v -coverprofile cover.out ./... endif .PHONY: test-scorecard diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 3cb81c12b..d8c86710a 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -39,6 +39,7 @@ package controllers_test import ( "fmt" "go/build" + "os" "path/filepath" "testing" @@ -78,8 +79,10 @@ var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) By("bootstrapping test environment") - // TODO hardcoded version - openshiftPrefix := []string{build.Default.GOPATH, "pkg", "mod", "github.com", "openshift", "api@v0.0.0-20230406152840-ce21e3fe5da2"} + openshiftModVersion := os.Getenv("OPENSHIFT_API_MOD_VERSION") + Expect(openshiftModVersion).ToNot(BeEmpty(), "OPENSHIFT_API_MOD_VERSION environment variable must be set") + openshiftPrefix := []string{build.Default.GOPATH, "pkg", "mod", "github.com", "openshift", + "api@" + openshiftModVersion} testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{ filepath.Join("..", "..", "config", "crd", "bases"), From e581c199bf47a5cc3eb814cf69563289e02cebe5 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Thu, 15 Jun 2023 13:53:26 -0400 Subject: [PATCH 05/19] clean up test --- internal/controllers/reconciler_test.go | 35 ++++--------------------- 1 file changed, 5 insertions(+), 30 deletions(-) diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index d1ff3ffeb..25c790679 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -2231,6 +2231,11 @@ func (c *controllerTest) commonTestsWithManager() { Expect(k8sClient).NotTo(BeNil()) // Create openshift-config namespace first + // Don't include this with other objects, since those will be deleted + // after the test. EnvTest doesn't support namespace deletion, so an + // attempt to delete openshift-config will prevent us from using it + // for subsequent tests. + // See: https://book.kubebuilder.io/reference/envtest.html#namespace-usage-limitation ns := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "openshift-config", @@ -2239,41 +2244,11 @@ func (c *controllerTest) commonTestsWithManager() { err = k8sClient.Create(context.Background(), ns) Expect(ctrlclient.IgnoreAlreadyExists(err)).ToNot(HaveOccurred()) - // Create namespaces first - // nsGeneratedNameMap := map[string]string{} - // otherObjs := []ctrlclient.Object{} for _, obj := range t.objs { - // if ns, ok := obj.(*corev1.Namespace); ok { - // saveName := ns.Name - // ns.GenerateName = ns.Name + "-" - // ns.Name = "" - // fmt.Printf("Creating: %v\n", ns) // XXX - // err := k8sClient.Create(context.TODO(), obj) - // Expect(err).ToNot(HaveOccurred()) - // fmt.Println("Namespace: " + ns.Name) // XXX - // nsGeneratedNameMap[saveName] = ns.Name - // } else { - // otherObjs = append(otherObjs, obj) - // } - // } - // // Create other objects while substituting namespace - // for _, obj := range otherObjs { - // if len(obj.GetNamespace()) > 0 { - // Expect(nsGeneratedNameMap).To(HaveKey(obj.GetNamespace())) - // ns := nsGeneratedNameMap[obj.GetNamespace()] - // obj.SetNamespace(ns) - // } fmt.Printf("Creating: %v\n", obj) // XXX err := k8sClient.Create(context.TODO(), obj) Expect(err).ToNot(HaveOccurred()) } - // // Replace namespace variables with generated names - // Expect(nsGeneratedNameMap).To(HaveKey(t.Namespace)) - // t.Namespace = nsGeneratedNameMap[t.Namespace] - // Expect(nsGeneratedNameMap).To(HaveKey(otherNamespace)) - // otherNamespace = nsGeneratedNameMap[otherNamespace] - // t.TargetNamespaces = []string{t.Namespace} - // t.watchNamespaces = []string{t.Namespace} t.Client = k8sClient controller, err := c.constructorFunc(t.newReconcilerConfig(scheme.Scheme, t.Client)) From a7e4944d267334219a6dbe1232a97831aadb8b7d Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Thu, 2 Nov 2023 09:47:21 -0400 Subject: [PATCH 06/19] Create HTTP proxy for communicating with Insights --- Makefile | 22 +- config/default/image_tag_patch.yaml | 2 + config/insights/insights_patch.yaml | 17 + config/insights/kustomization.yaml | 5 + config/manager/kustomization.yaml | 4 +- config/manager/manager.yaml | 4 + config/rbac/role.yaml | 14 + hack/insights_patch.yaml.in | 17 + internal/controllers/insights/apicast.go | 119 ++++++ internal/controllers/insights/insights.go | 395 ++++++++++++++++++ .../insights/insights_controller.go | 175 ++++++++ internal/controllers/openshift.go | 70 ---- internal/controllers/reconciler.go | 62 +-- internal/main.go | 56 ++- 14 files changed, 824 insertions(+), 138 deletions(-) create mode 100644 config/insights/insights_patch.yaml create mode 100644 config/insights/kustomization.yaml create mode 100644 hack/insights_patch.yaml.in create mode 100644 internal/controllers/insights/apicast.go create mode 100644 internal/controllers/insights/insights.go create mode 100644 internal/controllers/insights/insights_controller.go diff --git a/Makefile b/Makefile index 21b2542cc..c30393723 100644 --- a/Makefile +++ b/Makefile @@ -114,6 +114,19 @@ ifneq ("$(wildcard $(GINKGO))","") GO_TEST="$(GINKGO)" -cover -output-dir=. endif +# Optional Red Hat Insights integration +ENABLE_INSIGHTS ?= false +ifeq ($(ENABLE_INSIGHTS), true) +KUSTOMIZE_DIR ?= config/insights +INSIGHTS_PROXY_NAMESPACE ?= quay.io/3scale +INSIGHTS_PROXY_NAME ?= apicast +INSIGHTS_PROXY_VERSION ?= insights-01 +export INSIGHTS_PROXY_IMG ?= $(INSIGHTS_PROXY_NAMESPACE)/$(INSIGHTS_PROXY_NAME):$(INSIGHTS_PROXY_VERSION) +export INSIGHTS_BACKEND ?= cert.console.redhat.com +else +KUSTOMIZE_DIR ?= config/default +endif + .PHONY: all all: manager @@ -203,12 +216,12 @@ predeploy: .PHONY: print_deploy_config print_deploy_config: predeploy - $(KUSTOMIZE) build config/default + $(KUSTOMIZE) build $(KUSTOMIZE_DIR) # Deploy controller in the configured Kubernetes cluster in ~/.kube/config .PHONY: deploy deploy: check_cert_manager manifests kustomize predeploy - $(KUSTOMIZE) build config/default | $(CLUSTER_CLIENT) apply -f - + $(KUSTOMIZE) build $(KUSTOMIZE_DIR) | $(CLUSTER_CLIENT) apply -f - ifeq ($(DISABLE_SERVICE_TLS), true) @echo "Disabling TLS for in-cluster communication between Services" @$(CLUSTER_CLIENT) -n $(DEPLOY_NAMESPACE) set env deployment/cryostat-operator-controller-manager DISABLE_SERVICE_TLS=true @@ -219,7 +232,7 @@ endif undeploy: - $(CLUSTER_CLIENT) delete --ignore-not-found=$(ignore-not-found) -f config/samples/operator_v1beta1_cryostat.yaml - $(CLUSTER_CLIENT) delete --ignore-not-found=$(ignore-not-found) -f config/samples/operator_v1beta1_clustercryostat.yaml - - $(KUSTOMIZE) build config/default | $(CLUSTER_CLIENT) delete --ignore-not-found=$(ignore-not-found) -f - + - $(KUSTOMIZE) build $(KUSTOMIZE_DIR) | $(CLUSTER_CLIENT) delete --ignore-not-found=$(ignore-not-found) -f - # Generate manifests e.g. CRD, RBAC etc. .PHONY: manifests @@ -227,6 +240,9 @@ manifests: controller-gen $(CONTROLLER_GEN) rbac:roleName=role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases envsubst < hack/image_tag_patch.yaml.in > config/default/image_tag_patch.yaml envsubst < hack/image_pull_patch.yaml.in > config/default/image_pull_patch.yaml +ifeq ($(ENABLE_INSIGHTS), true) + envsubst < hack/insights_patch.yaml.in > config/insights/insights_patch.yaml +endif # Run go fmt against code .PHONY: fmt diff --git a/config/default/image_tag_patch.yaml b/config/default/image_tag_patch.yaml index 32f0912c3..cc7abe470 100644 --- a/config/default/image_tag_patch.yaml +++ b/config/default/image_tag_patch.yaml @@ -17,3 +17,5 @@ spec: value: "quay.io/cryostat/cryostat-grafana-dashboard:latest" - name: RELATED_IMAGE_REPORTS value: "quay.io/cryostat/cryostat-reports:latest" + - name: RELATED_IMAGE_INSIGHTS_PROXY + value: "quay.io/3scale/apicast:insights-01" diff --git a/config/insights/insights_patch.yaml b/config/insights/insights_patch.yaml new file mode 100644 index 000000000..7ad971737 --- /dev/null +++ b/config/insights/insights_patch.yaml @@ -0,0 +1,17 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager + namespace: system +spec: + template: + spec: + containers: + - name: manager + env: + - name: RELATED_IMAGE_INSIGHTS_PROXY + value: "quay.io/3scale/apicast:insights-01" + - name: INSIGHTS_ENABLED + value: "true" + - name: INSIGHTS_BACKEND_DOMAIN + value: "cert.console.redhat.com" diff --git a/config/insights/kustomization.yaml b/config/insights/kustomization.yaml new file mode 100644 index 000000000..cf37b360f --- /dev/null +++ b/config/insights/kustomization.yaml @@ -0,0 +1,5 @@ +resources: +- ../default + +patchesStrategicMerge: +- insights_patch.yaml diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index c105afc04..591f3a3ce 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -12,5 +12,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization images: - name: controller - newName: quay.io/cryostat/cryostat-operator - newTag: 2.4.0-dev + newName: quay.io/ebaron/cryostat-operator + newTag: insights-test-09 diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index d231596e5..277a0b7df 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -49,6 +49,10 @@ spec: env: - name: WATCH_NAMESPACE value: "" + - name: NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace resources: limits: cpu: 1000m diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 0e3fb7ddb..aff9b7313 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -33,6 +33,13 @@ rules: - replicationcontrollers verbs: - get +- apiGroups: + - "" + resources: + - secrets + - services + verbs: + - '*' - apiGroups: - apps resources: @@ -42,6 +49,13 @@ rules: - statefulsets verbs: - '*' +- apiGroups: + - apps + resources: + - deployments + - deployments/finalizers + verbs: + - '*' - apiGroups: - apps.openshift.io resources: diff --git a/hack/insights_patch.yaml.in b/hack/insights_patch.yaml.in new file mode 100644 index 000000000..dd30bf6ed --- /dev/null +++ b/hack/insights_patch.yaml.in @@ -0,0 +1,17 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager + namespace: system +spec: + template: + spec: + containers: + - name: manager + env: + - name: RELATED_IMAGE_INSIGHTS_PROXY + value: "${INSIGHTS_PROXY_IMG}" + - name: INSIGHTS_ENABLED + value: "true" + - name: INSIGHTS_BACKEND_DOMAIN + value: "${INSIGHTS_BACKEND}" diff --git a/internal/controllers/insights/apicast.go b/internal/controllers/insights/apicast.go new file mode 100644 index 000000000..d600805c9 --- /dev/null +++ b/internal/controllers/insights/apicast.go @@ -0,0 +1,119 @@ +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package insights + +import ( + "bytes" + "text/template" +) + +type apiCastConfigParams struct { + FrontendDomains string + BackendInsightsDomain string + HeaderValue string + ProxyDomain string +} + +var apiCastConfigTemplate = template.Must(template.New("").Parse(`{ + "services": [ + { + "id": "1", + "backend_version": "1", + "proxy": { + "hosts": [{{ .FrontendDomains }}], + "api_backend": "https://{{ .BackendInsightsDomain }}:443/", + "backend": { "endpoint": "http://127.0.0.1:8081", "host": "backend" }, + "policy_chain": [ + { + "name": "default_credentials", + "version": "builtin", + "configuration": { + "auth_type": "user_key", + "user_key": "dummy_key" + } + }, + {{- if .ProxyDomain }} + { + "name": "apicast.policy.http_proxy", + "configuration": { + "https_proxy": "http://{{ .ProxyDomain }}/", + "http_proxy": "http://{{ .ProxyDomain }}/" + } + }, + {{- end }} + { + "name": "headers", + "version": "builtin", + "configuration": { + "request": [ + { + "op": "set", + "header": "Authorization", + "value_type": "plain", + "value": "{{ .HeaderValue }}" + } + ] + } + }, + { + "name": "apicast.policy.apicast" + } + ], + "proxy_rules": [ + { + "http_method": "POST", + "pattern": "/", + "metric_system_name": "hits", + "delta": 1, + "parameters": [], + "querystring_parameters": {} + } + ] + } + } + ] +}`)) + +func getAPICastConfig(params *apiCastConfigParams) (*string, error) { + buf := &bytes.Buffer{} + err := apiCastConfigTemplate.Execute(buf, params) + if err != nil { + return nil, err + } + result := buf.String() + return &result, nil +} diff --git a/internal/controllers/insights/insights.go b/internal/controllers/insights/insights.go new file mode 100644 index 000000000..c2df8e1bd --- /dev/null +++ b/internal/controllers/insights/insights.go @@ -0,0 +1,395 @@ +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package insights + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +func (r *InsightsReconciler) reconcileInsights(ctx context.Context) error { + err := r.reconcilePullSecret(ctx) + if err != nil { + return err + } + err = r.reconcileProxyDeployment(ctx) + if err != nil { + return err + } + return r.reconcileProxyService(ctx) +} + +func (r *InsightsReconciler) reconcilePullSecret(ctx context.Context) error { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ProxySecretName, + Namespace: r.Namespace, + }, + } + owner := &appsv1.Deployment{} + err := r.Client.Get(ctx, types.NamespacedName{Name: OperatorDeploymentName, + Namespace: r.Namespace}, owner) + if err != nil { + return err + } + + token, err := r.getTokenFromPullSecret(ctx) + if err != nil { + // TODO warn instead of fail? + return err + } + + // TODO convert to APICast secret + params := &apiCastConfigParams{ + FrontendDomains: fmt.Sprintf("\"%s\",\"%s.%s.svc.cluster.local\"", ProxyServiceName, ProxyServiceName, r.Namespace), + BackendInsightsDomain: r.backendDomain, + ProxyDomain: r.proxyDomain, + HeaderValue: *token, + } + config, err := getAPICastConfig(params) + if err != nil { + return err + } + + return r.createOrUpdateSecret(ctx, secret, owner, func() error { + if secret.StringData == nil { + secret.StringData = map[string]string{} + } + secret.StringData["config.json"] = *config + return nil + }) +} + +func (r *InsightsReconciler) reconcileProxyDeployment(ctx context.Context) error { + deploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: ProxyDeploymentName, + Namespace: r.Namespace, + }, + } + owner := &appsv1.Deployment{} + err := r.Client.Get(ctx, types.NamespacedName{Name: OperatorDeploymentName, + Namespace: r.Namespace}, owner) + if err != nil { + return err + } + + return r.createOrUpdateDeployment(ctx, deploy, owner) +} + +func (r *InsightsReconciler) reconcileProxyService(ctx context.Context) error { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: ProxyServiceName, + Namespace: r.Namespace, + }, + } + owner := &appsv1.Deployment{} + err := r.Client.Get(ctx, types.NamespacedName{Name: OperatorDeploymentName, + Namespace: r.Namespace}, owner) + if err != nil { + return err + } + + return r.createOrUpdateService(ctx, svc, owner) +} + +func (r *InsightsReconciler) getTokenFromPullSecret(ctx context.Context) (*string, error) { + // Get the global pull secret + pullSecret := &corev1.Secret{} + err := r.Client.Get(ctx, types.NamespacedName{Namespace: "openshift-config", Name: "pull-secret"}, pullSecret) + if err != nil { + return nil, err + } + + // Look for the .dockerconfigjson key within it + dockerConfigRaw, pres := pullSecret.Data[corev1.DockerConfigJsonKey] + if !pres { + return nil, fmt.Errorf("no %s key present in pull secret", corev1.DockerConfigJsonKey) + } + + fmt.Println("dockerconfig: " + string(dockerConfigRaw)) + // Unmarshal the .dockerconfigjson into a struct + dockerConfig := struct { + Auths map[string]struct { + Auth string `json:"auth"` + } `json:"auths"` + }{} + err = json.Unmarshal(dockerConfigRaw, &dockerConfig) + if err != nil { + return nil, err + } + + // Look for the "cloud.openshift.com" auth + openshiftAuth, pres := dockerConfig.Auths["cloud.openshift.com"] + if !pres { + return nil, errors.New("no \"cloud.openshift.com\" auth within pull secret") + } + + token := strings.TrimSpace(openshiftAuth.Auth) + if strings.Contains(token, "\n") || strings.Contains(token, "\r") { + return nil, fmt.Errorf("invalid cloud.openshift.com token") + } + return &token, nil +} + +// FIXME dedup +func (r *InsightsReconciler) createOrUpdateSecret(ctx context.Context, secret *corev1.Secret, owner metav1.Object, + delegate controllerutil.MutateFn) error { + op, err := controllerutil.CreateOrUpdate(ctx, r.Client, secret, func() error { + // Set the Cryostat CR as controller + if err := controllerutil.SetControllerReference(owner, secret, r.Scheme); err != nil { + return err + } + // Call the delegate for secret-specific mutations + return delegate() + }) + if err != nil { + return err + } + r.Log.Info(fmt.Sprintf("Secret %s", op), "name", secret.Name, "namespace", secret.Namespace) + return nil +} + +// TODO dedup +func (r *InsightsReconciler) createOrUpdateDeployment(ctx context.Context, deploy *appsv1.Deployment, owner metav1.Object) error { + op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deploy, func() error { + labels := map[string]string{"app": ProxyDeploymentName} + annotations := map[string]string{} + mergeLabelsAndAnnotations(&deploy.ObjectMeta, labels, annotations) + // Set the Cryostat CR as controller + if err := controllerutil.SetControllerReference(owner, deploy, r.Scheme); err != nil { + return err + } + // Immutable, only updated when the deployment is created + if deploy.CreationTimestamp.IsZero() { + // Selector is immutable, avoid modifying if possible + deploy.Spec.Selector = &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": ProxyDeploymentName, // FIXME Does this need to be "deployment"? + }, + } + } + // TODO handle selector modified case? + + // Update pod template spec to propagate any changes from Cryostat CR + deploy.Spec.Template.Spec = *r.getPodSpec() + // Update pod template metadata + mergeLabelsAndAnnotations(&deploy.Spec.Template.ObjectMeta, labels, annotations) + return nil + }) + if err != nil { + return err + } + r.Log.Info(fmt.Sprintf("Deployment %s", op), "name", deploy.Name, "namespace", deploy.Namespace) + return nil +} + +func (r *InsightsReconciler) createOrUpdateService(ctx context.Context, svc *corev1.Service, owner metav1.Object) error { + op, err := controllerutil.CreateOrUpdate(ctx, r.Client, svc, func() error { + // Update labels and annotations + labels := map[string]string{"app": ProxyDeploymentName} + annotations := map[string]string{} + mergeLabelsAndAnnotations(&svc.ObjectMeta, labels, annotations) + + // Set the Cryostat CR as controller + if err := controllerutil.SetControllerReference(owner, svc, r.Scheme); err != nil { + return err + } + // Update the service type + svc.Spec.Type = corev1.ServiceTypeClusterIP + svc.Spec.Selector = map[string]string{ + "app": ProxyDeploymentName, + } + svc.Spec.Ports = []corev1.ServicePort{ + { + Name: "proxy", + Port: 8080, + TargetPort: intstr.FromString("proxy"), + }, + { + Name: "management", + Port: 8090, + TargetPort: intstr.FromString("management"), + }, + } + return nil + }) + if err != nil { + return err + } + r.Log.Info(fmt.Sprintf("Service %s", op), "name", svc.Name, "namespace", svc.Namespace) + return nil +} + +// TODO dedup +// ALL capability to drop for restricted pod security. See: +// https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted +const capabilityAll corev1.Capability = "ALL" + +func (r *InsightsReconciler) getPodSpec() *corev1.PodSpec { + privEscalation := false + nonRoot := true + readOnlyMode := int32(0440) + + return &corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: ProxyDeploymentName, + Image: r.proxyImageTag, + Env: []corev1.EnvVar{ + { + Name: "THREESCALE_CONFIG_FILE", + Value: "/tmp/gateway-configuration-volume/config.json", + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "gateway-configuration-volume", + MountPath: "/tmp/gateway-configuration-volume", + ReadOnly: true, + }, + }, + Ports: []corev1.ContainerPort{ + { + Name: "proxy", + ContainerPort: 8080, + }, + { + Name: "management", + ContainerPort: 8090, + }, + { + Name: "metrics", + ContainerPort: 9421, + }, + }, + Resources: corev1.ResourceRequirements{}, // TODO + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: &privEscalation, + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{capabilityAll}, + }, + }, + LivenessProbe: &corev1.Probe{ + InitialDelaySeconds: 10, + TimeoutSeconds: 5, + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/status/live", + Port: intstr.FromInt(8090), + }, + }, + }, + ReadinessProbe: &corev1.Probe{ + InitialDelaySeconds: 15, + PeriodSeconds: 30, + TimeoutSeconds: 5, + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/status/ready", + Port: intstr.FromInt(8090), + }, + }, + }, + }, + }, + Volumes: []corev1.Volume{ // TODO detect change and redeploy + { + Name: "gateway-configuration-volume", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: ProxySecretName, + Items: []corev1.KeyToPath{ + { + Key: "config.json", + Path: "config.json", + Mode: &readOnlyMode, + }, + }, + }, + }, + }, + }, + SecurityContext: &corev1.PodSecurityContext{ + RunAsNonRoot: &nonRoot, + SeccompProfile: seccompProfile(true), + }, + } +} + +// TODO dedup or remove +func seccompProfile(openshift bool) *corev1.SeccompProfile { + // For backward-compatibility with OpenShift < 4.11, + // leave the seccompProfile empty. In OpenShift >= 4.11, + // the restricted-v2 SCC will populate it for us. + if openshift { + return nil + } + return &corev1.SeccompProfile{ + Type: corev1.SeccompProfileTypeRuntimeDefault, + } +} + +// TODO dedup +func mergeLabelsAndAnnotations(dest *metav1.ObjectMeta, srcLabels, srcAnnotations map[string]string) { + // Check and create labels/annotations map if absent + if dest.Labels == nil { + dest.Labels = map[string]string{} + } + if dest.Annotations == nil { + dest.Annotations = map[string]string{} + } + + // Merge labels and annotations, preferring those in the source + for k, v := range srcLabels { + dest.Labels[k] = v + } + for k, v := range srcAnnotations { + dest.Annotations[k] = v + } +} diff --git a/internal/controllers/insights/insights_controller.go b/internal/controllers/insights/insights_controller.go new file mode 100644 index 000000000..6f1f97a5a --- /dev/null +++ b/internal/controllers/insights/insights_controller.go @@ -0,0 +1,175 @@ +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package insights + +import ( + "context" + "errors" + "fmt" + "os" + + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/cryostatio/cryostat-operator/internal/controllers/common" + "github.com/go-logr/logr" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" +) + +// TODO +type InsightsReconciler struct { + *InsightsReconcilerConfig + backendDomain string + proxyDomain string + proxyImageTag string +} + +type InsightsReconcilerConfig struct { + client.Client + Log logr.Logger + Scheme *runtime.Scheme + Namespace string + common.ReconcilerTLS +} + +const ( + // TODO move to constants, share + OperatorDeploymentName = "cryostat-operator-controller-manager" + ProxyDeploymentName = "insights-proxy" + ProxyServiceName = ProxyDeploymentName + ProxySecretName = "apicastconf" + EnvInsightsBackendDomain = "INSIGHTS_BACKEND_DOMAIN" // TODO Kustomize? + EnvInsightsProxyDomain = "INSIGHTS_PROXY_DOMAIN" + EnvInsightsEnabled = "INSIGHTS_ENABLED" + // Environment variable to override the Insights proxy image + EnvInsightsProxyImageTag = "RELATED_IMAGE_INSIGHTS_PROXY" +) + +func NewInsightsReconciler(config *InsightsReconcilerConfig) (*InsightsReconciler, error) { + backendDomain := config.GetEnv(EnvInsightsBackendDomain) + if len(backendDomain) == 0 { + return nil, errors.New("no backend domain provided for Insights") + } + imageTag := os.Getenv(EnvInsightsProxyImageTag) + if len(imageTag) == 0 { + return nil, errors.New("no proxy image tag provided for Insights") + } + proxyDomain := config.GetEnv(EnvInsightsProxyDomain) + + return &InsightsReconciler{ + InsightsReconcilerConfig: config, + backendDomain: backendDomain, + proxyDomain: proxyDomain, + proxyImageTag: imageTag, + }, nil +} + +// +kubebuilder:rbac:groups=apps,resources=deployments;deployments/finalizers,verbs=* +// +kubebuilder:rbac:groups="",resources=services;secrets,verbs=* + +// Reconcile processes a Cryostat CR and manages a Cryostat installation accordingly +func (r *InsightsReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { + reqLogger := r.Log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name) + + reqLogger.Info("Reconciling Insights Proxy") + + // FIXME probably can remove this + if request.Name != ProxyDeploymentName || request.Namespace != r.Namespace { + reqLogger.Error(nil, "Invalid request") + } + // Reconcile all Insights support + err := r.reconcileInsights(ctx) + if err != nil { + return reconcile.Result{}, err + } + return reconcile.Result{}, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *InsightsReconciler) SetupWithManager(mgr ctrl.Manager) error { + c := ctrl.NewControllerManagedBy(mgr). + Named("insights"). + Watches(&source.Kind{Type: &corev1.Secret{}}, + handler.EnqueueRequestsFromMapFunc(r.isPullSecretOrProxyConfig), // TODO extract handlers and unit test + builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})). + Watches(&source.Kind{Type: &appsv1.Deployment{}}, + handler.EnqueueRequestsFromMapFunc(r.isProxyDeployment), + builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})). + Watches(&source.Kind{Type: &corev1.Service{}}, + handler.EnqueueRequestsFromMapFunc(r.isProxyService), + builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})) + return c.Complete(r) +} + +func (r *InsightsReconciler) isPullSecretOrProxyConfig(secret client.Object) []reconcile.Request { + if !(secret.GetNamespace() == "openshift-config" && secret.GetName() == "pull-secret") && + !(secret.GetNamespace() == r.Namespace && secret.GetName() == ProxySecretName) { + fmt.Printf("GOT SECRET %s/%s\n", secret.GetNamespace(), secret.GetName()) + return nil + } + return r.proxyDeploymentRequest() +} + +func (r *InsightsReconciler) isProxyDeployment(deploy client.Object) []reconcile.Request { + if deploy.GetNamespace() != r.Namespace || deploy.GetName() != ProxyDeploymentName { + fmt.Printf("GOT DEPLOYMENT %s/%s\n", deploy.GetNamespace(), deploy.GetName()) + return nil + } + return r.proxyDeploymentRequest() +} + +func (r *InsightsReconciler) isProxyService(svc client.Object) []reconcile.Request { + if svc.GetNamespace() != r.Namespace || svc.GetName() != ProxyServiceName { + fmt.Printf("GOT SERVICE %s/%s\n", svc.GetNamespace(), svc.GetName()) + return nil + } + return r.proxyDeploymentRequest() +} + +func (r *InsightsReconciler) proxyDeploymentRequest() []reconcile.Request { + req := reconcile.Request{NamespacedName: types.NamespacedName{Namespace: r.Namespace, Name: ProxyDeploymentName}} + return []reconcile.Request{req} +} diff --git a/internal/controllers/openshift.go b/internal/controllers/openshift.go index 55c58e53d..39a4170f8 100644 --- a/internal/controllers/openshift.go +++ b/internal/controllers/openshift.go @@ -38,18 +38,14 @@ package controllers import ( "context" - "encoding/json" - "errors" "fmt" "regexp" - "strings" "github.com/cryostatio/cryostat-operator/internal/controllers/common" "github.com/cryostatio/cryostat-operator/internal/controllers/model" "github.com/go-logr/logr" configv1 "github.com/openshift/api/config/v1" consolev1 "github.com/openshift/api/console/v1" - corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -64,10 +60,6 @@ func (r *Reconciler) reconcileOpenShift(ctx context.Context, cr *model.CryostatI if err != nil { return err } - err = r.reconcilePullSecret(ctx, cr) - if err != nil { - return err - } return r.addCorsAllowedOriginIfNotPresent(ctx, cr) } @@ -211,65 +203,3 @@ func (r *Reconciler) deleteCorsAllowedOrigins(ctx context.Context, cr *model.Cry reqLogger.Info("Removed from APIServer CORS allowed origins") return nil } - -func (r *Reconciler) reconcilePullSecret(ctx context.Context, cr *model.CryostatInstance) error { - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: cr.Name + "-insights-token", - Namespace: cr.InstallNamespace, - }, - } - - token, err := r.getTokenFromPullSecret(ctx) - if err != nil { - // TODO warn instead of fail? - return err - } - - return r.createOrUpdateSecret(ctx, secret, cr.Object, func() error { - if secret.StringData == nil { - secret.StringData = map[string]string{} - } - secret.StringData["token"] = *token - return nil - }) -} - -func (r *Reconciler) getTokenFromPullSecret(ctx context.Context) (*string, error) { - // Get the global pull secret - pullSecret := &corev1.Secret{} - err := r.Client.Get(ctx, types.NamespacedName{Namespace: "openshift-config", Name: "pull-secret"}, pullSecret) - if err != nil { - return nil, err - } - - // Look for the .dockerconfigjson key within it - dockerConfigRaw, pres := pullSecret.Data[corev1.DockerConfigJsonKey] - if !pres { - return nil, fmt.Errorf("no %s key present in pull secret", corev1.DockerConfigJsonKey) - } - - fmt.Println("dockerconfig: " + string(dockerConfigRaw)) - // Unmarshal the .dockerconfigjson into a struct - dockerConfig := struct { - Auths map[string]struct { - Auth string `json:"auth"` - } `json:"auths"` - }{} - err = json.Unmarshal(dockerConfigRaw, &dockerConfig) - if err != nil { - return nil, err - } - - // Look for the "cloud.openshift.com" auth - openshiftAuth, pres := dockerConfig.Auths["cloud.openshift.com"] - if !pres { - return nil, errors.New("no \"cloud.openshift.com\" auth within pull secret") - } - - token := strings.TrimSpace(openshiftAuth.Auth) - if strings.Contains(token, "\n") || strings.Contains(token, "\r") { - return nil, fmt.Errorf("invalid cloud.openshift.com token") - } - return &token, nil -} diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 3e8c8ff11..b625bed91 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -65,14 +65,11 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/controller-runtime/pkg/source" ) // ReconcilerConfig contains common configuration parameters for @@ -86,6 +83,7 @@ type ReconcilerConfig struct { EventRecorder record.EventRecorder RESTMapper meta.RESTMapper Namespaces []string + Insights bool common.ReconcilerTLS } @@ -341,10 +339,7 @@ func (r *Reconciler) setupWithManager(mgr ctrl.Manager, impl reconcile.Reconcile } } return true - })). - Watches(&source.Kind{Type: &corev1.Secret{}}, - handler.EnqueueRequestsFromMapFunc(r.findObjectsForPullSecret), - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})) + })) // Watch for changes to secondary resources and requeue the owner Cryostat resources := []client.Object{&appsv1.Deployment{}, &corev1.Service{}, &corev1.Secret{}, &corev1.PersistentVolumeClaim{}, @@ -636,59 +631,6 @@ func mergeLabelsAndAnnotations(dest *metav1.ObjectMeta, srcLabels, srcAnnotation } } -func (r *Reconciler) findObjectsForPullSecret(secret client.Object) []reconcile.Request { - if secret.GetNamespace() != "openshift-config" || secret.GetName() != "pull-secret" { - fmt.Printf("GOT SECRET %s/%s\n", secret.GetNamespace(), secret.GetName()) - return nil - } - namespaces := []string{""} - if r.isNamespaced { - namespaces = r.Namespaces - } - - result := []reconcile.Request{} - for _, namespace := range namespaces { - instanceList, ok := r.listType.DeepCopyObject().(client.ObjectList) - if !ok { - r.Log.Error(fmt.Errorf("type %T is not an ObjectList", r.listType), - "could not allocate a list") - return []reconcile.Request{} - } - - listOps := &client.ListOptions{ - Namespace: namespace, - } - err := r.List(context.Background(), instanceList, listOps) - if err != nil { - r.Log.Error(err, "failed to list %T", r.objectType, "namespace", namespace) - return []reconcile.Request{} - } - - items, err := meta.ExtractList(instanceList) - if err != nil { - r.Log.Error(err, "could not get items from list") - return []reconcile.Request{} - } - requests := make([]reconcile.Request, len(items)) - for i, item := range items { - obj, err := meta.Accessor(item) - if err != nil { - r.Log.Error(err, "list contains an invalid item") - } - requests[i] = reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - }, - } - // XXX - r.Log.Info(fmt.Sprintf("Enqueueing %T %s for pull-secret change", r.objectType, types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}.String())) - } - result = append(result, requests...) - } - return result -} - func namespacesToSet(namespaces []string) map[string]struct{} { result := make(map[string]struct{}, len(namespaces)) for _, namespace := range namespaces { diff --git a/internal/main.go b/internal/main.go index 20ada7181..c339b0de0 100644 --- a/internal/main.go +++ b/internal/main.go @@ -48,6 +48,7 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + "github.com/go-logr/logr" configv1 "github.com/openshift/api/config/v1" consolev1 "github.com/openshift/api/console/v1" routev1 "github.com/openshift/api/route/v1" @@ -63,6 +64,7 @@ import ( operatorv1beta1 "github.com/cryostatio/cryostat-operator/api/v1beta1" "github.com/cryostatio/cryostat-operator/internal/controllers" "github.com/cryostatio/cryostat-operator/internal/controllers/common" + "github.com/cryostatio/cryostat-operator/internal/controllers/insights" // +kubebuilder:scaffold:imports ) @@ -163,8 +165,26 @@ func main() { setupLog.Info("did not find cert-manager installation") } + // Optionally enable Insights integration + insights := false + // TODO want some way to clean up without deleting operator. Maybe need a ConfigMap or CR owner instead of this deployment + if isInsightsEnabled() { + namespace := getOperatorNamespace() + // This will happen when running the operator locally + if len(namespace) == 0 { + setupLog.Info("Operator namespace not detected, disabling Insights integration") + } else { + err := createInsightsController(mgr, namespace, setupLog) + if err != nil { + setupLog.Error(err, "unable to add controller to manager", "controller", "Insights") + } else { + insights = true + } + } + } + config := newReconcilerConfig(mgr, "ClusterCryostat", "clustercryostat-controller", openShift, - certManager, namespaces) + certManager, namespaces, insights) clusterController, err := controllers.NewClusterCryostatReconciler(config) if err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterCryostat") @@ -175,7 +195,7 @@ func main() { os.Exit(1) } config = newReconcilerConfig(mgr, "Cryostat", "cryostat-controller", openShift, certManager, - namespaces) + namespaces, insights) controller, err := controllers.NewCryostatReconciler(config) if err != nil { setupLog.Error(err, "unable to create controller", "controller", "Cryostat") @@ -226,7 +246,7 @@ func isCertManagerInstalled(client discovery.DiscoveryInterface) (bool, error) { } func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName string, openShift bool, - certManager bool, namespaces []string) *controllers.ReconcilerConfig { + certManager bool, namespaces []string, insights bool) *controllers.ReconcilerConfig { return &controllers.ReconcilerConfig{ Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName(logName), @@ -236,8 +256,38 @@ func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName str EventRecorder: mgr.GetEventRecorderFor(eventRecorderName), RESTMapper: mgr.GetRESTMapper(), Namespaces: namespaces, + Insights: insights, + ReconcilerTLS: common.NewReconcilerTLS(&common.ReconcilerTLSConfig{ + Client: mgr.GetClient(), + }), + } +} + +func isInsightsEnabled() bool { + return strings.ToLower(os.Getenv(insights.EnvInsightsEnabled)) == "true" +} + +func getOperatorNamespace() string { + return os.Getenv("NAMESPACE") +} + +func createInsightsController(mgr ctrl.Manager, namespace string, log logr.Logger) error { + config := &insights.InsightsReconcilerConfig{ + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("Insights"), + Scheme: mgr.GetScheme(), + Namespace: namespace, ReconcilerTLS: common.NewReconcilerTLS(&common.ReconcilerTLSConfig{ Client: mgr.GetClient(), }), } + controller, err := insights.NewInsightsReconciler(config) + if err != nil { + return err + } + if err := controller.SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to add controller to manager", "controller", "Insights") + os.Exit(1) + } + return nil } From b35c923c27aa124d42311963a63b3ae4aeef1f3d Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Thu, 2 Nov 2023 20:06:33 -0400 Subject: [PATCH 07/19] Handle deletion case, add tests --- config/default/image_tag_patch.yaml | 2 - config/manager/kustomization.yaml | 4 +- config/rbac/role.yaml | 16 +- internal/controllers/common/common_utils.go | 30 +- .../resource_definitions.go | 12 +- internal/controllers/common/tls.go | 2 +- internal/controllers/constants/constants.go | 4 + .../controllers/cryostat_controller_test.go | 43 +++ internal/controllers/insights/insights.go | 93 ++---- .../insights/insights_controller.go | 21 +- .../insights/insights_controller_test.go | 193 +++++++++++ .../insights/insights_suite_test.go | 118 +++++++ internal/controllers/insights/setup.go | 161 ++++++++++ .../controllers/insights/test/resources.go | 304 ++++++++++++++++++ internal/controllers/insights/test/utils.go | 84 +++++ internal/controllers/pvc.go | 3 +- internal/controllers/reconciler.go | 66 ++-- internal/controllers/reconciler_test.go | 2 +- internal/main.go | 50 +-- 19 files changed, 1017 insertions(+), 191 deletions(-) create mode 100644 internal/controllers/insights/insights_controller_test.go create mode 100644 internal/controllers/insights/insights_suite_test.go create mode 100644 internal/controllers/insights/setup.go create mode 100644 internal/controllers/insights/test/resources.go create mode 100644 internal/controllers/insights/test/utils.go diff --git a/config/default/image_tag_patch.yaml b/config/default/image_tag_patch.yaml index cc7abe470..32f0912c3 100644 --- a/config/default/image_tag_patch.yaml +++ b/config/default/image_tag_patch.yaml @@ -17,5 +17,3 @@ spec: value: "quay.io/cryostat/cryostat-grafana-dashboard:latest" - name: RELATED_IMAGE_REPORTS value: "quay.io/cryostat/cryostat-reports:latest" - - name: RELATED_IMAGE_INSIGHTS_PROXY - value: "quay.io/3scale/apicast:insights-01" diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 591f3a3ce..c105afc04 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -12,5 +12,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization images: - name: controller - newName: quay.io/ebaron/cryostat-operator - newTag: insights-test-09 + newName: quay.io/cryostat/cryostat-operator + newTag: 2.4.0-dev diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index aff9b7313..25853c46d 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -5,6 +5,15 @@ metadata: creationTimestamp: null name: role rules: +- apiGroups: + - "" + resources: + - configmaps + - configmaps/finalizers + - secrets + - services + verbs: + - '*' - apiGroups: - "" resources: @@ -33,13 +42,6 @@ rules: - replicationcontrollers verbs: - get -- apiGroups: - - "" - resources: - - secrets - - services - verbs: - - '*' - apiGroups: - apps resources: diff --git a/internal/controllers/common/common_utils.go b/internal/controllers/common/common_utils.go index eb817f600..0229a82e1 100644 --- a/internal/controllers/common/common_utils.go +++ b/internal/controllers/common/common_utils.go @@ -45,6 +45,7 @@ import ( "strings" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -59,21 +60,21 @@ type OSUtils interface { GenPasswd(length int) string } -type defaultOSUtils struct{} +type DefaultOSUtils struct{} // GetEnv returns the value of the environment variable with the provided name. If no such // variable exists, the empty string is returned. -func (o *defaultOSUtils) GetEnv(name string) string { +func (o *DefaultOSUtils) GetEnv(name string) string { return os.Getenv(name) } // GetFileContents reads and returns the entire file contents specified by the path -func (o *defaultOSUtils) GetFileContents(path string) ([]byte, error) { +func (o *DefaultOSUtils) GetFileContents(path string) ([]byte, error) { return ioutil.ReadFile(path) } // GenPasswd generates a psuedorandom password of a given length. -func (o *defaultOSUtils) GenPasswd(length int) string { +func (o *DefaultOSUtils) GenPasswd(length int) string { rand.Seed(time.Now().UnixNano()) chars := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" b := make([]byte, length) @@ -91,3 +92,24 @@ func ClusterUniqueName(gvk *schema.GroupVersionKind, name string, namespace stri suffix := fmt.Sprintf("%x", sha256.Sum256([]byte(nn.String()))) return strings.ToLower(gvk.Kind) + "-" + suffix } + +// MergeLabelsAndAnnotations copies labels and annotations from a source +// to the destination ObjectMeta, overwriting any existing labels and +// annotations of the same key. +func MergeLabelsAndAnnotations(dest *metav1.ObjectMeta, srcLabels, srcAnnotations map[string]string) { + // Check and create labels/annotations map if absent + if dest.Labels == nil { + dest.Labels = map[string]string{} + } + if dest.Annotations == nil { + dest.Annotations = map[string]string{} + } + + // Merge labels and annotations, preferring those in the source + for k, v := range srcLabels { + dest.Labels[k] = v + } + for k, v := range srcAnnotations { + dest.Annotations[k] = v + } +} diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 743cbfb6f..20c1996d4 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -410,10 +410,6 @@ func NewReportContainerResource(cr *model.CryostatInstance) *corev1.ResourceRequ return resources } -// ALL capability to drop for restricted pod security. See: -// https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted -const capabilityAll corev1.Capability = "ALL" - func NewPodForReports(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, openshift bool) *corev1.PodSpec { resources := NewReportContainerResource(cr) cpus := resources.Requests.Cpu().Value() // Round to 1 if cpu request < 1000m @@ -515,7 +511,7 @@ func NewPodForReports(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLS containerSc = &corev1.SecurityContext{ AllowPrivilegeEscalation: &privEscalation, Capabilities: &corev1.Capabilities{ - Drop: []corev1.Capability{capabilityAll}, + Drop: []corev1.Capability{constants.CapabilityAll}, }, } } @@ -920,7 +916,7 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag containerSc = &corev1.SecurityContext{ AllowPrivilegeEscalation: &privEscalation, Capabilities: &corev1.Capabilities{ - Drop: []corev1.Capability{capabilityAll}, + Drop: []corev1.Capability{constants.CapabilityAll}, }, } } @@ -1010,7 +1006,7 @@ func NewGrafanaContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo containerSc = &corev1.SecurityContext{ AllowPrivilegeEscalation: &privEscalation, Capabilities: &corev1.Capabilities{ - Drop: []corev1.Capability{capabilityAll}, + Drop: []corev1.Capability{constants.CapabilityAll}, }, } } @@ -1070,7 +1066,7 @@ func NewJfrDatasourceContainer(cr *model.CryostatInstance, imageTag string) core containerSc = &corev1.SecurityContext{ AllowPrivilegeEscalation: &privEscalation, Capabilities: &corev1.Capabilities{ - Drop: []corev1.Capability{capabilityAll}, + Drop: []corev1.Capability{constants.CapabilityAll}, }, } } diff --git a/internal/controllers/common/tls.go b/internal/controllers/common/tls.go index 800206238..2da1680c3 100644 --- a/internal/controllers/common/tls.go +++ b/internal/controllers/common/tls.go @@ -81,7 +81,7 @@ const disableServiceTLS = "DISABLE_SERVICE_TLS" func NewReconcilerTLS(config *ReconcilerTLSConfig) ReconcilerTLS { configCopy := *config if config.OSUtils == nil { - configCopy.OSUtils = &defaultOSUtils{} + configCopy.OSUtils = &DefaultOSUtils{} } return &reconcilerTLS{ ReconcilerTLSConfig: &configCopy, diff --git a/internal/controllers/constants/constants.go b/internal/controllers/constants/constants.go index 5ffa45d08..c4da08453 100644 --- a/internal/controllers/constants/constants.go +++ b/internal/controllers/constants/constants.go @@ -38,6 +38,7 @@ package constants import ( certMeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + corev1 "k8s.io/api/core/v1" ) const ( @@ -53,4 +54,7 @@ const ( CAKey = certMeta.TLSCAKey // Hostname alias for loopback address, to be used for health checks HealthCheckHostname = "cryostat-health.local" + // ALL capability to drop for restricted pod security. See: + // https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted + CapabilityAll corev1.Capability = "ALL" ) diff --git a/internal/controllers/cryostat_controller_test.go b/internal/controllers/cryostat_controller_test.go index 96582170e..7caedba67 100644 --- a/internal/controllers/cryostat_controller_test.go +++ b/internal/controllers/cryostat_controller_test.go @@ -38,7 +38,11 @@ package controllers_test import ( "github.com/cryostatio/cryostat-operator/internal/controllers" + "github.com/cryostatio/cryostat-operator/internal/controllers/model" . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" ) var _ = Describe("CryostatController", func() { @@ -48,6 +52,45 @@ var _ = Describe("CryostatController", func() { } c.commonTests() + + Describe("filtering requests", func() { + Context("watches the configured namespace(s)", func() { + var t *cryostatTestInput + var filter predicate.Predicate + var cr *model.CryostatInstance + + BeforeEach(func() { + t = c.commonBeforeEach() + }) + JustBeforeEach(func() { + c.commonJustBeforeEach(t) + filter = controllers.NamespaceEventFilter(t.Client.Scheme(), t.watchNamespaces) + }) + Context("creating a CR in the watched namespace", func() { + BeforeEach(func() { + cr = t.NewCryostat() + }) + It("should reconcile the CR", func() { + result := filter.Create(event.CreateEvent{ + Object: cr.Object, + }) + Expect(result).To(BeTrue()) + }) + }) + Context("creating a CR in a non-watched namespace", func() { + BeforeEach(func() { + t.Namespace = "something-else" + cr = t.NewCryostat() + }) + It("should reconcile the CR", func() { + result := filter.Create(event.CreateEvent{ + Object: cr.Object, + }) + Expect(result).To(BeFalse()) + }) + }) + }) + }) }) func newCryostatController(config *controllers.ReconcilerConfig) (controllers.CommonReconciler, error) { diff --git a/internal/controllers/insights/insights.go b/internal/controllers/insights/insights.go index c2df8e1bd..6b98b6756 100644 --- a/internal/controllers/insights/insights.go +++ b/internal/controllers/insights/insights.go @@ -43,6 +43,8 @@ import ( "fmt" "strings" + "github.com/cryostatio/cryostat-operator/internal/controllers/common" + "github.com/cryostatio/cryostat-operator/internal/controllers/constants" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -70,8 +72,8 @@ func (r *InsightsReconciler) reconcilePullSecret(ctx context.Context) error { Namespace: r.Namespace, }, } - owner := &appsv1.Deployment{} - err := r.Client.Get(ctx, types.NamespacedName{Name: OperatorDeploymentName, + owner := &corev1.ConfigMap{} + err := r.Client.Get(ctx, types.NamespacedName{Name: InsightsConfigMapName, Namespace: r.Namespace}, owner) if err != nil { return err @@ -79,11 +81,9 @@ func (r *InsightsReconciler) reconcilePullSecret(ctx context.Context) error { token, err := r.getTokenFromPullSecret(ctx) if err != nil { - // TODO warn instead of fail? return err } - // TODO convert to APICast secret params := &apiCastConfigParams{ FrontendDomains: fmt.Sprintf("\"%s\",\"%s.%s.svc.cluster.local\"", ProxyServiceName, ProxyServiceName, r.Namespace), BackendInsightsDomain: r.backendDomain, @@ -95,13 +95,7 @@ func (r *InsightsReconciler) reconcilePullSecret(ctx context.Context) error { return err } - return r.createOrUpdateSecret(ctx, secret, owner, func() error { - if secret.StringData == nil { - secret.StringData = map[string]string{} - } - secret.StringData["config.json"] = *config - return nil - }) + return r.createOrUpdateProxySecret(ctx, secret, owner, *config) } func (r *InsightsReconciler) reconcileProxyDeployment(ctx context.Context) error { @@ -111,14 +105,14 @@ func (r *InsightsReconciler) reconcileProxyDeployment(ctx context.Context) error Namespace: r.Namespace, }, } - owner := &appsv1.Deployment{} - err := r.Client.Get(ctx, types.NamespacedName{Name: OperatorDeploymentName, + owner := &corev1.ConfigMap{} + err := r.Client.Get(ctx, types.NamespacedName{Name: InsightsConfigMapName, Namespace: r.Namespace}, owner) if err != nil { return err } - return r.createOrUpdateDeployment(ctx, deploy, owner) + return r.createOrUpdateProxyDeployment(ctx, deploy, owner) } func (r *InsightsReconciler) reconcileProxyService(ctx context.Context) error { @@ -128,14 +122,14 @@ func (r *InsightsReconciler) reconcileProxyService(ctx context.Context) error { Namespace: r.Namespace, }, } - owner := &appsv1.Deployment{} - err := r.Client.Get(ctx, types.NamespacedName{Name: OperatorDeploymentName, + owner := &corev1.ConfigMap{} + err := r.Client.Get(ctx, types.NamespacedName{Name: InsightsConfigMapName, Namespace: r.Namespace}, owner) if err != nil { return err } - return r.createOrUpdateService(ctx, svc, owner) + return r.createOrUpdateProxyService(ctx, svc, owner) } func (r *InsightsReconciler) getTokenFromPullSecret(ctx context.Context) (*string, error) { @@ -177,16 +171,19 @@ func (r *InsightsReconciler) getTokenFromPullSecret(ctx context.Context) (*strin return &token, nil } -// FIXME dedup -func (r *InsightsReconciler) createOrUpdateSecret(ctx context.Context, secret *corev1.Secret, owner metav1.Object, - delegate controllerutil.MutateFn) error { +func (r *InsightsReconciler) createOrUpdateProxySecret(ctx context.Context, secret *corev1.Secret, owner metav1.Object, + config string) error { op, err := controllerutil.CreateOrUpdate(ctx, r.Client, secret, func() error { - // Set the Cryostat CR as controller + // Set the config map as controller if err := controllerutil.SetControllerReference(owner, secret, r.Scheme); err != nil { return err } - // Call the delegate for secret-specific mutations - return delegate() + // Add the APICast config.json + if secret.StringData == nil { + secret.StringData = map[string]string{} + } + secret.StringData["config.json"] = config + return nil }) if err != nil { return err @@ -195,13 +192,12 @@ func (r *InsightsReconciler) createOrUpdateSecret(ctx context.Context, secret *c return nil } -// TODO dedup -func (r *InsightsReconciler) createOrUpdateDeployment(ctx context.Context, deploy *appsv1.Deployment, owner metav1.Object) error { +func (r *InsightsReconciler) createOrUpdateProxyDeployment(ctx context.Context, deploy *appsv1.Deployment, owner metav1.Object) error { op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deploy, func() error { labels := map[string]string{"app": ProxyDeploymentName} annotations := map[string]string{} - mergeLabelsAndAnnotations(&deploy.ObjectMeta, labels, annotations) - // Set the Cryostat CR as controller + common.MergeLabelsAndAnnotations(&deploy.ObjectMeta, labels, annotations) + // Set the config map as controller if err := controllerutil.SetControllerReference(owner, deploy, r.Scheme); err != nil { return err } @@ -210,16 +206,15 @@ func (r *InsightsReconciler) createOrUpdateDeployment(ctx context.Context, deplo // Selector is immutable, avoid modifying if possible deploy.Spec.Selector = &metav1.LabelSelector{ MatchLabels: map[string]string{ - "app": ProxyDeploymentName, // FIXME Does this need to be "deployment"? + "app": ProxyDeploymentName, }, } } - // TODO handle selector modified case? - // Update pod template spec to propagate any changes from Cryostat CR - deploy.Spec.Template.Spec = *r.getPodSpec() + // Update pod template spec + deploy.Spec.Template.Spec = *r.getProxyPodSpec() // Update pod template metadata - mergeLabelsAndAnnotations(&deploy.Spec.Template.ObjectMeta, labels, annotations) + common.MergeLabelsAndAnnotations(&deploy.Spec.Template.ObjectMeta, labels, annotations) return nil }) if err != nil { @@ -229,14 +224,14 @@ func (r *InsightsReconciler) createOrUpdateDeployment(ctx context.Context, deplo return nil } -func (r *InsightsReconciler) createOrUpdateService(ctx context.Context, svc *corev1.Service, owner metav1.Object) error { +func (r *InsightsReconciler) createOrUpdateProxyService(ctx context.Context, svc *corev1.Service, owner metav1.Object) error { op, err := controllerutil.CreateOrUpdate(ctx, r.Client, svc, func() error { // Update labels and annotations labels := map[string]string{"app": ProxyDeploymentName} annotations := map[string]string{} - mergeLabelsAndAnnotations(&svc.ObjectMeta, labels, annotations) + common.MergeLabelsAndAnnotations(&svc.ObjectMeta, labels, annotations) - // Set the Cryostat CR as controller + // Set the config map as controller if err := controllerutil.SetControllerReference(owner, svc, r.Scheme); err != nil { return err } @@ -266,12 +261,7 @@ func (r *InsightsReconciler) createOrUpdateService(ctx context.Context, svc *cor return nil } -// TODO dedup -// ALL capability to drop for restricted pod security. See: -// https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted -const capabilityAll corev1.Capability = "ALL" - -func (r *InsightsReconciler) getPodSpec() *corev1.PodSpec { +func (r *InsightsReconciler) getProxyPodSpec() *corev1.PodSpec { privEscalation := false nonRoot := true readOnlyMode := int32(0440) @@ -312,7 +302,7 @@ func (r *InsightsReconciler) getPodSpec() *corev1.PodSpec { SecurityContext: &corev1.SecurityContext{ AllowPrivilegeEscalation: &privEscalation, Capabilities: &corev1.Capabilities{ - Drop: []corev1.Capability{capabilityAll}, + Drop: []corev1.Capability{constants.CapabilityAll}, }, }, LivenessProbe: &corev1.Probe{ @@ -374,22 +364,3 @@ func seccompProfile(openshift bool) *corev1.SeccompProfile { Type: corev1.SeccompProfileTypeRuntimeDefault, } } - -// TODO dedup -func mergeLabelsAndAnnotations(dest *metav1.ObjectMeta, srcLabels, srcAnnotations map[string]string) { - // Check and create labels/annotations map if absent - if dest.Labels == nil { - dest.Labels = map[string]string{} - } - if dest.Annotations == nil { - dest.Annotations = map[string]string{} - } - - // Merge labels and annotations, preferring those in the source - for k, v := range srcLabels { - dest.Labels[k] = v - } - for k, v := range srcAnnotations { - dest.Annotations[k] = v - } -} diff --git a/internal/controllers/insights/insights_controller.go b/internal/controllers/insights/insights_controller.go index 6f1f97a5a..cce49f3ab 100644 --- a/internal/controllers/insights/insights_controller.go +++ b/internal/controllers/insights/insights_controller.go @@ -40,7 +40,6 @@ import ( "context" "errors" "fmt" - "os" ctrl "sigs.k8s.io/controller-runtime" @@ -51,10 +50,8 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" ) @@ -72,13 +69,14 @@ type InsightsReconcilerConfig struct { Log logr.Logger Scheme *runtime.Scheme Namespace string - common.ReconcilerTLS + common.OSUtils } const ( // TODO move to constants, share + InsightsConfigMapName = "insights-proxy" OperatorDeploymentName = "cryostat-operator-controller-manager" - ProxyDeploymentName = "insights-proxy" + ProxyDeploymentName = InsightsConfigMapName ProxyServiceName = ProxyDeploymentName ProxySecretName = "apicastconf" EnvInsightsBackendDomain = "INSIGHTS_BACKEND_DOMAIN" // TODO Kustomize? @@ -93,7 +91,7 @@ func NewInsightsReconciler(config *InsightsReconcilerConfig) (*InsightsReconcile if len(backendDomain) == 0 { return nil, errors.New("no backend domain provided for Insights") } - imageTag := os.Getenv(EnvInsightsProxyImageTag) + imageTag := config.GetEnv(EnvInsightsProxyImageTag) if len(imageTag) == 0 { return nil, errors.New("no proxy image tag provided for Insights") } @@ -108,7 +106,7 @@ func NewInsightsReconciler(config *InsightsReconcilerConfig) (*InsightsReconcile } // +kubebuilder:rbac:groups=apps,resources=deployments;deployments/finalizers,verbs=* -// +kubebuilder:rbac:groups="",resources=services;secrets,verbs=* +// +kubebuilder:rbac:groups="",resources=services;secrets;configmaps;configmaps/finalizers,verbs=* // Reconcile processes a Cryostat CR and manages a Cryostat installation accordingly func (r *InsightsReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { @@ -133,14 +131,11 @@ func (r *InsightsReconciler) SetupWithManager(mgr ctrl.Manager) error { c := ctrl.NewControllerManagedBy(mgr). Named("insights"). Watches(&source.Kind{Type: &corev1.Secret{}}, - handler.EnqueueRequestsFromMapFunc(r.isPullSecretOrProxyConfig), // TODO extract handlers and unit test - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})). + handler.EnqueueRequestsFromMapFunc(r.isPullSecretOrProxyConfig)). // TODO extract handlers and unit test Watches(&source.Kind{Type: &appsv1.Deployment{}}, - handler.EnqueueRequestsFromMapFunc(r.isProxyDeployment), - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})). + handler.EnqueueRequestsFromMapFunc(r.isProxyDeployment)). Watches(&source.Kind{Type: &corev1.Service{}}, - handler.EnqueueRequestsFromMapFunc(r.isProxyService), - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})) + handler.EnqueueRequestsFromMapFunc(r.isProxyService)) return c.Complete(r) } diff --git a/internal/controllers/insights/insights_controller_test.go b/internal/controllers/insights/insights_controller_test.go new file mode 100644 index 000000000..ecfaab65c --- /dev/null +++ b/internal/controllers/insights/insights_controller_test.go @@ -0,0 +1,193 @@ +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package insights_test + +import ( + "context" + + "github.com/cryostatio/cryostat-operator/internal/controllers/insights" + insightstest "github.com/cryostatio/cryostat-operator/internal/controllers/insights/test" + "github.com/cryostatio/cryostat-operator/internal/test" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +type insightsTestInput struct { + client ctrlclient.Client + controller *insights.InsightsReconciler + objs []ctrlclient.Object + *insightstest.TestUtilsConfig + *insightstest.InsightsTestResources +} + +var _ = Describe("InsightsController", func() { + var t *insightsTestInput + + Describe("reconciling a request", func() { + + BeforeEach(func() { + t = &insightsTestInput{ + TestUtilsConfig: &insightstest.TestUtilsConfig{ + EnvInsightsEnabled: &[]bool{true}[0], + EnvInsightsBackendDomain: &[]string{"insights.example.com"}[0], + EnvInsightsProxyImageTag: &[]string{"example.com/proxy:latest"}[0], + }, + InsightsTestResources: &insightstest.InsightsTestResources{ + TestResources: &test.TestResources{ + Namespace: "test", + }, + }, + } + t.objs = []ctrlclient.Object{ + t.NewNamespace(), + t.NewGlobalPullSecret(), + t.NewOperatorDeployment(), + t.NewProxyConfigMap(), + } + }) + + JustBeforeEach(func() { + s := test.NewTestScheme() + logger := zap.New() + logf.SetLogger(logger) + + // Set a CreationTimestamp for created objects to match a real API server + // TODO When using envtest instead of fake client, this is probably no longer needed + err := test.SetCreationTimestamp(t.objs...) + Expect(err).ToNot(HaveOccurred()) + t.client = fake.NewClientBuilder().WithScheme(s).WithObjects(t.objs...).Build() + + config := &insights.InsightsReconcilerConfig{ + Client: test.NewClientWithTimestamp(test.NewTestClient(t.client, t.TestResources)), + Scheme: s, + Log: logger, + Namespace: t.Namespace, + OSUtils: insightstest.NewTestOSUtils(t.TestUtilsConfig), + } + t.controller, err = insights.NewInsightsReconciler(config) + Expect(err).ToNot(HaveOccurred()) + }) + + Context("successfully creates required resources", func() { + Context("with defaults", func() { + JustBeforeEach(func() { + result, err := t.reconcile() + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + }) + It("should create the APICast config secret", func() { + expected := t.NewInsightsProxySecret() + actual := &corev1.Secret{} + t.client.Get(context.Background(), types.NamespacedName{ + Name: expected.Name, + Namespace: expected.Namespace, + }, actual) + + Expect(actual.Labels).To(Equal(expected.Labels)) + Expect(actual.Annotations).To(Equal(expected.Annotations)) + Expect(metav1.IsControlledBy(actual, t.NewProxyConfigMap())).To(BeTrue()) + Expect(actual.StringData).To(HaveLen(1)) + Expect(actual.StringData).To(HaveKey("config.json")) + Expect(actual.StringData["config.json"]).To(MatchJSON(expected.StringData["config.json"])) + }) + It("should create the proxy deployment", func() { + expected := t.NewInsightsProxyDeployment() + actual := &appsv1.Deployment{} + t.client.Get(context.Background(), types.NamespacedName{ + Name: expected.Name, + Namespace: expected.Namespace, + }, actual) + + Expect(actual.Labels).To(Equal(expected.Labels)) + Expect(actual.Annotations).To(Equal(expected.Annotations)) + Expect(metav1.IsControlledBy(actual, t.NewProxyConfigMap())).To(BeTrue()) + Expect(actual.Spec.Selector).To(Equal(expected.Spec.Selector)) + + expectedTemplate := expected.Spec.Template + actualTemplate := actual.Spec.Template + Expect(actualTemplate.Labels).To(Equal(expectedTemplate.Labels)) + Expect(actualTemplate.Annotations).To(Equal(expectedTemplate.Annotations)) + Expect(actualTemplate.Spec.SecurityContext).To(Equal(expectedTemplate.Spec.SecurityContext)) + Expect(actualTemplate.Spec.Volumes).To(Equal(expectedTemplate.Spec.Volumes)) + + Expect(actualTemplate.Spec.Containers).To(HaveLen(1)) + expectedContainer := expectedTemplate.Spec.Containers[0] + actualContainer := actualTemplate.Spec.Containers[0] + Expect(actualContainer.Ports).To(ConsistOf(expectedContainer.Ports)) + Expect(actualContainer.Env).To(ConsistOf(expectedContainer.Env)) + Expect(actualContainer.EnvFrom).To(ConsistOf(expectedContainer.EnvFrom)) + Expect(actualContainer.VolumeMounts).To(ConsistOf(expectedContainer.VolumeMounts)) + Expect(actualContainer.LivenessProbe).To(Equal(expectedContainer.LivenessProbe)) + Expect(actualContainer.StartupProbe).To(Equal(expectedContainer.StartupProbe)) + Expect(actualContainer.SecurityContext).To(Equal(expectedContainer.SecurityContext)) + Expect(actualContainer.Resources).To(Equal(expectedContainer.Resources)) + }) + It("should create the proxy service", func() { + expected := t.NewInsightsProxyService() + actual := &corev1.Service{} + t.client.Get(context.Background(), types.NamespacedName{ + Name: expected.Name, + Namespace: expected.Namespace, + }, actual) + + Expect(actual.Labels).To(Equal(expected.Labels)) + Expect(actual.Annotations).To(Equal(expected.Annotations)) + Expect(metav1.IsControlledBy(actual, t.NewProxyConfigMap())).To(BeTrue()) + + Expect(actual.Spec.Selector).To(Equal(expected.Spec.Selector)) + Expect(actual.Spec.Type).To(Equal(expected.Spec.Type)) + Expect(actual.Spec.Ports).To(ConsistOf(expected.Spec.Ports)) + }) + }) + }) + }) +}) + +func (t *insightsTestInput) reconcile() (reconcile.Result, error) { + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "insights-proxy", Namespace: t.Namespace}} + return t.controller.Reconcile(context.Background(), req) +} diff --git a/internal/controllers/insights/insights_suite_test.go b/internal/controllers/insights/insights_suite_test.go new file mode 100644 index 000000000..a9f2fa8a1 --- /dev/null +++ b/internal/controllers/insights/insights_suite_test.go @@ -0,0 +1,118 @@ +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package insights_test + +import ( + "fmt" + "path/filepath" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + operatorv1beta1 "github.com/cryostatio/cryostat-operator/api/v1beta1" + + certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + configv1 "github.com/openshift/api/config/v1" + consolev1 "github.com/openshift/api/console/v1" + routev1 "github.com/openshift/api/route/v1" + // +kubebuilder:scaffold:imports +) + +// These tests use Ginkgo (BDD-style Go testing framework). Refer to +// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. + +var cfg *rest.Config +var k8sClient client.Client +var testEnv *envtest.Environment + +func TestInsights(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Insights Suite") +} + +var _ = BeforeSuite(func() { + logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + + By("bootstrapping test environment") + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{ + filepath.Join("..", "..", "..", "config", "crd", "bases"), + }, + ErrorIfCRDPathMissing: true, + } + fmt.Println(testEnv.CRDDirectoryPaths) + + var err error + // cfg is defined in this file globally. + cfg, err = testEnv.Start() + Expect(err).NotTo(HaveOccurred()) + Expect(cfg).NotTo(BeNil()) + + err = operatorv1beta1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + + err = certv1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + + err = consolev1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + + err = routev1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + + err = configv1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + + // +kubebuilder:scaffold:scheme + + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(err).NotTo(HaveOccurred()) + Expect(k8sClient).NotTo(BeNil()) +}) + +var _ = AfterSuite(func() { + By("tearing down the test environment") + err := testEnv.Stop() + Expect(err).NotTo(HaveOccurred()) +}) diff --git a/internal/controllers/insights/setup.go b/internal/controllers/insights/setup.go new file mode 100644 index 000000000..55b493437 --- /dev/null +++ b/internal/controllers/insights/setup.go @@ -0,0 +1,161 @@ +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package insights + +import ( + "context" + "strings" + + "github.com/cryostatio/cryostat-operator/internal/controllers/common" + "github.com/go-logr/logr" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +type InsightsIntegration struct { + common.OSUtils +} + +func NewInsightsIntegration() *InsightsIntegration { + return &InsightsIntegration{ + OSUtils: &common.DefaultOSUtils{}, + } +} + +func (i *InsightsIntegration) Setup(mgr ctrl.Manager, log logr.Logger) (bool, error) { + enabled := false + namespace := i.getOperatorNamespace() + // This will happen when running the operator locally + if len(namespace) == 0 { + log.Info("Operator namespace not detected, disabling Insights integration") + return false, nil + } + + ctx := context.Background() + if i.isInsightsEnabled() { + err := i.createInsightsController(mgr, namespace, log) + if err != nil { + log.Error(err, "unable to add controller to manager", "controller", "Insights") + return false, err + } + // Create a Config Map to be used as a parent of all Insights Proxy related objects + err = i.createConfigMap(ctx, mgr, namespace) + if err != nil { + log.Error(err, "failed to create config map for Insights") + return false, err + } + enabled = true + } else { + // Delete any previously created Config Map (and its children) + err := i.deleteConfigMap(ctx, mgr, namespace) + if err != nil { + log.Error(err, "failed to delete config map for Insights") + return false, err + } + + } + return enabled, nil +} + +func (i *InsightsIntegration) isInsightsEnabled() bool { + return strings.ToLower(i.GetEnv(EnvInsightsEnabled)) == "true" +} + +func (i *InsightsIntegration) getOperatorNamespace() string { + return i.GetEnv("NAMESPACE") +} + +func (i *InsightsIntegration) createInsightsController(mgr ctrl.Manager, namespace string, log logr.Logger) error { + config := &InsightsReconcilerConfig{ + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("Insights"), + Scheme: mgr.GetScheme(), + Namespace: namespace, + OSUtils: i.OSUtils, + } + controller, err := NewInsightsReconciler(config) + if err != nil { + return err + } + if err := controller.SetupWithManager(mgr); err != nil { + return err + } + return nil +} + +func (i *InsightsIntegration) createConfigMap(ctx context.Context, mgr ctrl.Manager, namespace string) error { + // The config map should be owned by the operator deployment to ensure it and its descendants are garbage collected + owner := &appsv1.Deployment{} + err := mgr.GetAPIReader().Get(ctx, types.NamespacedName{Name: OperatorDeploymentName, Namespace: namespace}, owner) + if err != nil { + return err + } + + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: InsightsConfigMapName, + Namespace: namespace, + }, + } + err = controllerutil.SetControllerReference(owner, cm, mgr.GetScheme()) + if err != nil { + return err + } + + err = mgr.GetClient().Create(ctx, cm, &client.CreateOptions{}) + // This may already exist if the pod restarted + return client.IgnoreAlreadyExists(err) +} + +func (i *InsightsIntegration) deleteConfigMap(ctx context.Context, mgr ctrl.Manager, namespace string) error { + // Children will be garbage collected + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: InsightsConfigMapName, + Namespace: namespace, + }, + } + + err := mgr.GetClient().Delete(ctx, cm, &client.DeleteOptions{}) + // This may not exist if no config map was previously created + return client.IgnoreNotFound(err) +} diff --git a/internal/controllers/insights/test/resources.go b/internal/controllers/insights/test/resources.go new file mode 100644 index 000000000..20349bc52 --- /dev/null +++ b/internal/controllers/insights/test/resources.go @@ -0,0 +1,304 @@ +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package test + +import ( + "fmt" + + "github.com/cryostatio/cryostat-operator/internal/test" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +type InsightsTestResources struct { + *test.TestResources +} + +func (r *InsightsTestResources) NewGlobalPullSecret() *corev1.Secret { + config := `{"auths":{"example.com":{"auth":"hello"},"cloud.openshift.com":{"auth":"world"}}}` + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pull-secret", + Namespace: "openshift-config", + }, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(config), + }, + } +} + +func (r *InsightsTestResources) NewOperatorDeployment() *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cryostat-operator-controller-manager", + Namespace: r.Namespace, + }, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "control-plane": "controller-manager", + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "control-plane": "controller-manager", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "manager", + Image: "example.com/operator:latest", + }, + }, + }, + }, + }, + } +} + +func (r *InsightsTestResources) NewProxyConfigMap() *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "insights-proxy", + Namespace: r.Namespace, + }, + } +} + +func (r *InsightsTestResources) NewInsightsProxySecret() *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "apicastconf", + Namespace: r.Namespace, + }, + StringData: map[string]string{ + "config.json": fmt.Sprintf(`{ + "services": [ + { + "id": "1", + "backend_version": "1", + "proxy": { + "hosts": ["insights-proxy","insights-proxy.%s.svc.cluster.local"], + "api_backend": "https://insights.example.com:443/", + "backend": { "endpoint": "http://127.0.0.1:8081", "host": "backend" }, + "policy_chain": [ + { + "name": "default_credentials", + "version": "builtin", + "configuration": { + "auth_type": "user_key", + "user_key": "dummy_key" + } + }, + { + "name": "headers", + "version": "builtin", + "configuration": { + "request": [ + { + "op": "set", + "header": "Authorization", + "value_type": "plain", + "value": "world" + } + ] + } + }, + { + "name": "apicast.policy.apicast" + } + ], + "proxy_rules": [ + { + "http_method": "POST", + "pattern": "/", + "metric_system_name": "hits", + "delta": 1, + "parameters": [], + "querystring_parameters": {} + } + ] + } + } + ] + }`, r.Namespace), + }, + } +} + +func (r *InsightsTestResources) NewInsightsProxyDeployment() *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "insights-proxy", + Namespace: r.Namespace, + Labels: map[string]string{ + "app": "insights-proxy", + }, + }, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "insights-proxy", + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "insights-proxy", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "insights-proxy", + Image: "example.com/proxy:latest", + Env: []corev1.EnvVar{ + { + Name: "THREESCALE_CONFIG_FILE", + Value: "/tmp/gateway-configuration-volume/config.json", + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "gateway-configuration-volume", + MountPath: "/tmp/gateway-configuration-volume", + ReadOnly: true, + }, + }, + Ports: []corev1.ContainerPort{ + { + Name: "proxy", + ContainerPort: 8080, + }, + { + Name: "management", + ContainerPort: 8090, + }, + { + Name: "metrics", + ContainerPort: 9421, + }, + }, + Resources: corev1.ResourceRequirements{}, // TODO + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: &[]bool{false}[0], + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + }, + LivenessProbe: &corev1.Probe{ + InitialDelaySeconds: 10, + TimeoutSeconds: 5, + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/status/live", + Port: intstr.FromInt(8090), + }, + }, + }, + ReadinessProbe: &corev1.Probe{ + InitialDelaySeconds: 15, + PeriodSeconds: 30, + TimeoutSeconds: 5, + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/status/ready", + Port: intstr.FromInt(8090), + }, + }, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "gateway-configuration-volume", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "apicastconf", + Items: []corev1.KeyToPath{ + { + Key: "config.json", + Path: "config.json", + Mode: &[]int32{0440}[0], + }, + }, + }, + }, + }, + }, + SecurityContext: &corev1.PodSecurityContext{ + RunAsNonRoot: &[]bool{true}[0], + }, + }, + }, + }, + } +} + +func (r *InsightsTestResources) NewInsightsProxyService() *corev1.Service { + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "insights-proxy", + Namespace: r.Namespace, + Labels: map[string]string{ + "app": "insights-proxy", + }, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Selector: map[string]string{ + "app": "insights-proxy", + }, + Ports: []corev1.ServicePort{ + { + Name: "proxy", + Port: 8080, + TargetPort: intstr.FromString("proxy"), + }, + { + Name: "management", + Port: 8090, + TargetPort: intstr.FromString("management"), + }, + }, + }, + } +} diff --git a/internal/controllers/insights/test/utils.go b/internal/controllers/insights/test/utils.go new file mode 100644 index 000000000..be6767a7d --- /dev/null +++ b/internal/controllers/insights/test/utils.go @@ -0,0 +1,84 @@ +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package test + +import ( + "strconv" +) + +// TestUtilsConfig groups parameters used to create a test OSUtils +type TestUtilsConfig struct { + EnvInsightsEnabled *bool + EnvInsightsProxyImageTag *string + EnvInsightsBackendDomain *string + EnvInsightsProxyDomain *string +} + +type testOSUtils struct { + envs map[string]string +} + +func NewTestOSUtils(config *TestUtilsConfig) *testOSUtils { + envs := map[string]string{} + if config.EnvInsightsEnabled != nil { + envs["DISABLE_SERVICE_TLS"] = strconv.FormatBool(*config.EnvInsightsEnabled) + } + if config.EnvInsightsProxyImageTag != nil { + envs["RELATED_IMAGE_INSIGHTS_PROXY"] = *config.EnvInsightsProxyImageTag + } + if config.EnvInsightsBackendDomain != nil { + envs["INSIGHTS_BACKEND_DOMAIN"] = *config.EnvInsightsBackendDomain + } + if config.EnvInsightsProxyDomain != nil { + envs["INSIGHTS_PROXY_DOMAIN"] = *config.EnvInsightsProxyDomain + } + return &testOSUtils{envs: envs} +} + +func (o *testOSUtils) GetFileContents(path string) ([]byte, error) { + // Unused + return nil, nil +} + +func (o *testOSUtils) GetEnv(name string) string { + return o.envs[name] +} + +func (o *testOSUtils) GenPasswd(length int) string { + // Unused + return "" +} diff --git a/internal/controllers/pvc.go b/internal/controllers/pvc.go index 0c4501cb6..42269b99b 100644 --- a/internal/controllers/pvc.go +++ b/internal/controllers/pvc.go @@ -41,6 +41,7 @@ import ( "fmt" operatorv1beta1 "github.com/cryostatio/cryostat-operator/api/v1beta1" + "github.com/cryostatio/cryostat-operator/internal/controllers/common" "github.com/cryostatio/cryostat-operator/internal/controllers/model" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" @@ -86,7 +87,7 @@ func (r *Reconciler) createOrUpdatePVC(ctx context.Context, pvc *corev1.Persiste owner metav1.Object, config *operatorv1beta1.PersistentVolumeClaimConfig) error { op, err := controllerutil.CreateOrUpdate(ctx, r.Client, pvc, func() error { // Merge labels and annotations to prevent overriding any set by Kubernetes - mergeLabelsAndAnnotations(&pvc.ObjectMeta, config.Labels, config.Annotations) + common.MergeLabelsAndAnnotations(&pvc.ObjectMeta, config.Labels, config.Annotations) // Set the Cryostat CR as controller if err := controllerutil.SetControllerReference(owner, pvc, r.Scheme); err != nil { diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index b625bed91..7f636fbeb 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -98,7 +98,6 @@ type CommonReconciler interface { type Reconciler struct { *ReconcilerConfig objectType client.Object - listType client.ObjectList isNamespaced bool gvk *schema.GroupVersionKind } @@ -156,7 +155,6 @@ func newReconciler(config *ReconcilerConfig, objType client.Object, listType cli return &Reconciler{ ReconcilerConfig: config, objectType: objType, - listType: listType, isNamespaced: isNamespaced, gvk: &gvk, }, nil @@ -314,32 +312,28 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance) return reconcile.Result{}, nil } +func NamespaceEventFilter(scheme *runtime.Scheme, namespaceList []string) predicate.Predicate { + namespaces := namespacesToSet(namespaceList) + return predicate.NewPredicateFuncs(func(object client.Object) bool { + // Restrict watch for namespaced objects to specified namespaces + if len(object.GetNamespace()) > 0 { + _, pres := namespaces[object.GetNamespace()] + if !pres { + return false + } + } + return true + }) +} + func (r *Reconciler) setupWithManager(mgr ctrl.Manager, impl reconcile.Reconciler) error { - namespaces := namespacesToSet(r.Namespaces) c := ctrl.NewControllerManagedBy(mgr). - For(r.objectType). + For(r.objectType) + + if r.isNamespaced { // TODO remove this once only AllNamespace mode is supported - WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool { - // Restrict watch for namespaced CRs to specified namespaces - fmt.Println("KIND: " + object.GetObjectKind().GroupVersionKind().GroupKind().String()) // XXX - gk := object.GetObjectKind().GroupVersionKind().GroupKind() - if gk.Empty() { - gvk, err := apiutil.GVKForObject(object, mgr.GetScheme()) - if err != nil { - r.Log.Info("failed to find GVK for %T %s/%s", object, object.GetNamespace(), object.GetName()) - return false - } - gk = gvk.GroupKind() - fmt.Println("KIND after lookup: " + gvk.GroupKind().String()) // XXX - } - if gk == r.gvk.GroupKind() && len(object.GetNamespace()) > 0 { - _, pres := namespaces[object.GetNamespace()] - if !pres { - return false - } - } - return true - })) + c = c.WithEventFilter(NamespaceEventFilter(mgr.GetScheme(), r.Namespaces)) + } // Watch for changes to secondary resources and requeue the owner Cryostat resources := []client.Object{&appsv1.Deployment{}, &corev1.Service{}, &corev1.Secret{}, &corev1.PersistentVolumeClaim{}, @@ -511,7 +505,7 @@ func (r *Reconciler) createOrUpdateDeployment(ctx context.Context, deploy *appsv op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deploy, func() error { // TODO consider managing labels and annotations using CRD // Merge any required labels and annotations - mergeLabelsAndAnnotations(&deploy.ObjectMeta, deployCopy.Labels, deployCopy.Annotations) + common.MergeLabelsAndAnnotations(&deploy.ObjectMeta, deployCopy.Labels, deployCopy.Annotations) // Set the Cryostat CR as controller if err := controllerutil.SetControllerReference(owner, deploy, r.Scheme); err != nil { return err @@ -530,7 +524,7 @@ func (r *Reconciler) createOrUpdateDeployment(ctx context.Context, deploy *appsv // Update pod template spec to propagate any changes from Cryostat CR deploy.Spec.Template.Spec = deployCopy.Spec.Template.Spec // Update pod template metadata - mergeLabelsAndAnnotations(&deploy.Spec.Template.ObjectMeta, deployCopy.Spec.Template.Labels, + common.MergeLabelsAndAnnotations(&deploy.Spec.Template.ObjectMeta, deployCopy.Spec.Template.Labels, deployCopy.Spec.Template.Annotations) return nil }) @@ -613,24 +607,6 @@ func findDeployCondition(conditions []appsv1.DeploymentCondition, condType appsv return nil } -func mergeLabelsAndAnnotations(dest *metav1.ObjectMeta, srcLabels, srcAnnotations map[string]string) { - // Check and create labels/annotations map if absent - if dest.Labels == nil { - dest.Labels = map[string]string{} - } - if dest.Annotations == nil { - dest.Annotations = map[string]string{} - } - - // Merge labels and annotations, preferring those in the source - for k, v := range srcLabels { - dest.Labels[k] = v - } - for k, v := range srcAnnotations { - dest.Annotations[k] = v - } -} - func namespacesToSet(namespaces []string) map[string]struct{} { result := make(map[string]struct{}, len(namespaces)) for _, namespace := range namespaces { diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 25c790679..0f42827b7 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -234,7 +234,7 @@ func expectSuccessful(t **cryostatTestInput) { func (c *controllerTest) commonTests() { c.commonTestsWithoutManager() - c.commonTestsWithManager() + //c.commonTestsWithManager() } func (c *controllerTest) commonTestsWithoutManager() { diff --git a/internal/main.go b/internal/main.go index c339b0de0..600f9916a 100644 --- a/internal/main.go +++ b/internal/main.go @@ -48,7 +48,6 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - "github.com/go-logr/logr" configv1 "github.com/openshift/api/config/v1" consolev1 "github.com/openshift/api/console/v1" routev1 "github.com/openshift/api/route/v1" @@ -165,22 +164,10 @@ func main() { setupLog.Info("did not find cert-manager installation") } - // Optionally enable Insights integration - insights := false - // TODO want some way to clean up without deleting operator. Maybe need a ConfigMap or CR owner instead of this deployment - if isInsightsEnabled() { - namespace := getOperatorNamespace() - // This will happen when running the operator locally - if len(namespace) == 0 { - setupLog.Info("Operator namespace not detected, disabling Insights integration") - } else { - err := createInsightsController(mgr, namespace, setupLog) - if err != nil { - setupLog.Error(err, "unable to add controller to manager", "controller", "Insights") - } else { - insights = true - } - } + // Optionally enable Insights integration. Will only be enabled if INSIGHTS_ENABLED is true + insights, err := insights.NewInsightsIntegration().Setup(mgr, setupLog) + if err != nil { + setupLog.Error(err, "failed to set up Insights integration") } config := newReconcilerConfig(mgr, "ClusterCryostat", "clustercryostat-controller", openShift, @@ -262,32 +249,3 @@ func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName str }), } } - -func isInsightsEnabled() bool { - return strings.ToLower(os.Getenv(insights.EnvInsightsEnabled)) == "true" -} - -func getOperatorNamespace() string { - return os.Getenv("NAMESPACE") -} - -func createInsightsController(mgr ctrl.Manager, namespace string, log logr.Logger) error { - config := &insights.InsightsReconcilerConfig{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("Insights"), - Scheme: mgr.GetScheme(), - Namespace: namespace, - ReconcilerTLS: common.NewReconcilerTLS(&common.ReconcilerTLSConfig{ - Client: mgr.GetClient(), - }), - } - controller, err := insights.NewInsightsReconciler(config) - if err != nil { - return err - } - if err := controller.SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to add controller to manager", "controller", "Insights") - os.Exit(1) - } - return nil -} From 57c82a0bb235c43de6bdc2b77fa775c624cac718 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Fri, 3 Nov 2023 19:28:57 -0400 Subject: [PATCH 08/19] Set INSIGHTS_PROXY, additional testing --- .../resource_definitions.go | 18 +- .../controllers/cryostat_controller_test.go | 1 + .../insights/insights_controller.go | 13 +- .../insights/insights_controller_test.go | 134 +++++++++++++- .../insights/insights_controller_unit_test.go | 171 ++++++++++++++++++ internal/controllers/insights/setup.go | 71 +++++--- internal/controllers/insights/test/manager.go | 90 +++++++++ internal/controllers/insights/test/utils.go | 6 +- internal/controllers/reconciler.go | 7 +- internal/controllers/reconciler_test.go | 76 ++------ internal/main.go | 11 +- internal/test/resources.go | 9 + 12 files changed, 500 insertions(+), 107 deletions(-) create mode 100644 internal/controllers/insights/insights_controller_unit_test.go create mode 100644 internal/controllers/insights/test/manager.go diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 20c1996d4..7b1069ccf 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -62,9 +62,10 @@ type ImageTags struct { } type ServiceSpecs struct { - CoreURL *url.URL - GrafanaURL *url.URL - ReportsURL *url.URL + CoreURL *url.URL + GrafanaURL *url.URL + ReportsURL *url.URL + InsightsURL *url.URL } // TLSConfig contains TLS-related information useful when creating other objects @@ -691,6 +692,17 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag envs = append(envs, subprocessReportHeapEnv...) } + // Define INSIGHTS_PROXY URL if Insights integration is enabled + if specs.InsightsURL != nil { + insightsEnvs := []corev1.EnvVar{ + { + Name: "INSIGHTS_PROXY", + Value: specs.InsightsURL.String(), + }, + } + envs = append(envs, insightsEnvs...) + } + if cr.Spec.MaxWsConnections != 0 { maxWsConnections := strconv.Itoa(int(cr.Spec.MaxWsConnections)) maxWsConnectionsEnv := []corev1.EnvVar{ diff --git a/internal/controllers/cryostat_controller_test.go b/internal/controllers/cryostat_controller_test.go index 7caedba67..8e1a5e5af 100644 --- a/internal/controllers/cryostat_controller_test.go +++ b/internal/controllers/cryostat_controller_test.go @@ -61,6 +61,7 @@ var _ = Describe("CryostatController", func() { BeforeEach(func() { t = c.commonBeforeEach() + t.watchNamespaces = []string{t.Namespace} }) JustBeforeEach(func() { c.commonJustBeforeEach(t) diff --git a/internal/controllers/insights/insights_controller.go b/internal/controllers/insights/insights_controller.go index cce49f3ab..c243a7ab5 100644 --- a/internal/controllers/insights/insights_controller.go +++ b/internal/controllers/insights/insights_controller.go @@ -39,7 +39,6 @@ package insights import ( "context" "errors" - "fmt" ctrl "sigs.k8s.io/controller-runtime" @@ -79,7 +78,7 @@ const ( ProxyDeploymentName = InsightsConfigMapName ProxyServiceName = ProxyDeploymentName ProxySecretName = "apicastconf" - EnvInsightsBackendDomain = "INSIGHTS_BACKEND_DOMAIN" // TODO Kustomize? + EnvInsightsBackendDomain = "INSIGHTS_BACKEND_DOMAIN" EnvInsightsProxyDomain = "INSIGHTS_PROXY_DOMAIN" EnvInsightsEnabled = "INSIGHTS_ENABLED" // Environment variable to override the Insights proxy image @@ -111,13 +110,8 @@ func NewInsightsReconciler(config *InsightsReconcilerConfig) (*InsightsReconcile // Reconcile processes a Cryostat CR and manages a Cryostat installation accordingly func (r *InsightsReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { reqLogger := r.Log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name) - reqLogger.Info("Reconciling Insights Proxy") - // FIXME probably can remove this - if request.Name != ProxyDeploymentName || request.Namespace != r.Namespace { - reqLogger.Error(nil, "Invalid request") - } // Reconcile all Insights support err := r.reconcileInsights(ctx) if err != nil { @@ -131,7 +125,7 @@ func (r *InsightsReconciler) SetupWithManager(mgr ctrl.Manager) error { c := ctrl.NewControllerManagedBy(mgr). Named("insights"). Watches(&source.Kind{Type: &corev1.Secret{}}, - handler.EnqueueRequestsFromMapFunc(r.isPullSecretOrProxyConfig)). // TODO extract handlers and unit test + handler.EnqueueRequestsFromMapFunc(r.isPullSecretOrProxyConfig)). Watches(&source.Kind{Type: &appsv1.Deployment{}}, handler.EnqueueRequestsFromMapFunc(r.isProxyDeployment)). Watches(&source.Kind{Type: &corev1.Service{}}, @@ -142,7 +136,6 @@ func (r *InsightsReconciler) SetupWithManager(mgr ctrl.Manager) error { func (r *InsightsReconciler) isPullSecretOrProxyConfig(secret client.Object) []reconcile.Request { if !(secret.GetNamespace() == "openshift-config" && secret.GetName() == "pull-secret") && !(secret.GetNamespace() == r.Namespace && secret.GetName() == ProxySecretName) { - fmt.Printf("GOT SECRET %s/%s\n", secret.GetNamespace(), secret.GetName()) return nil } return r.proxyDeploymentRequest() @@ -150,7 +143,6 @@ func (r *InsightsReconciler) isPullSecretOrProxyConfig(secret client.Object) []r func (r *InsightsReconciler) isProxyDeployment(deploy client.Object) []reconcile.Request { if deploy.GetNamespace() != r.Namespace || deploy.GetName() != ProxyDeploymentName { - fmt.Printf("GOT DEPLOYMENT %s/%s\n", deploy.GetNamespace(), deploy.GetName()) return nil } return r.proxyDeploymentRequest() @@ -158,7 +150,6 @@ func (r *InsightsReconciler) isProxyDeployment(deploy client.Object) []reconcile func (r *InsightsReconciler) isProxyService(svc client.Object) []reconcile.Request { if svc.GetNamespace() != r.Namespace || svc.GetName() != ProxyServiceName { - fmt.Printf("GOT SERVICE %s/%s\n", svc.GetNamespace(), svc.GetName()) return nil } return r.proxyDeploymentRequest() diff --git a/internal/controllers/insights/insights_controller_test.go b/internal/controllers/insights/insights_controller_test.go index ecfaab65c..9c97df444 100644 --- a/internal/controllers/insights/insights_controller_test.go +++ b/internal/controllers/insights/insights_controller_test.go @@ -47,6 +47,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -67,6 +68,130 @@ type insightsTestInput struct { var _ = Describe("InsightsController", func() { var t *insightsTestInput + Describe("setting up", func() { + var integration *insights.InsightsIntegration + + BeforeEach(func() { + t = &insightsTestInput{ + TestUtilsConfig: &insightstest.TestUtilsConfig{ + EnvInsightsEnabled: &[]bool{true}[0], + EnvInsightsBackendDomain: &[]string{"insights.example.com"}[0], + EnvInsightsProxyImageTag: &[]string{"example.com/proxy:latest"}[0], + EnvNamespace: &[]string{"test"}[0], + }, + InsightsTestResources: &insightstest.InsightsTestResources{ + TestResources: &test.TestResources{ + Namespace: "test", + }, + }, + } + t.objs = []ctrlclient.Object{ + t.NewNamespace(), + t.NewGlobalPullSecret(), + t.NewOperatorDeployment(), + } + }) + + JustBeforeEach(func() { + s := test.NewTestScheme() + logger := zap.New() + logf.SetLogger(logger) + + // Set a CreationTimestamp for created objects to match a real API server + // TODO When using envtest instead of fake client, this is probably no longer needed + err := test.SetCreationTimestamp(t.objs...) + Expect(err).ToNot(HaveOccurred()) + t.client = fake.NewClientBuilder().WithScheme(s).WithObjects(t.objs...).Build() + + manager := insightstest.NewFakeManager(test.NewClientWithTimestamp(test.NewTestClient(t.client, t.TestResources)), + s, &logger) + integration = insights.NewInsightsIntegration(manager, &logger) + integration.OSUtils = insightstest.NewTestOSUtils(t.TestUtilsConfig) + }) + + Context("with defaults", func() { + It("should return proxy URL", func() { + result, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + Expect(result).ToNot(BeNil()) + Expect(result.String()).To(Equal("http://insights-proxy.test.svc.cluster.local")) + }) + + It("should create config map", func() { + _, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + + expected := t.NewProxyConfigMap() + actual := &corev1.ConfigMap{} + err = t.client.Get(context.Background(), types.NamespacedName{ + Name: expected.Name, + Namespace: expected.Namespace, + }, actual) + Expect(err).ToNot(HaveOccurred()) + + Expect(actual.Labels).To(Equal(expected.Labels)) + Expect(actual.Annotations).To(Equal(expected.Annotations)) + Expect(metav1.IsControlledBy(actual, t.NewOperatorDeployment())).To(BeTrue()) + Expect(actual.Data).To(BeEmpty()) + }) + }) + + Context("with Insights disabled", func() { + BeforeEach(func() { + t.EnvInsightsEnabled = &[]bool{false}[0] + t.objs = append(t.objs, + t.NewProxyConfigMap(), + ) + }) + + It("should return nil", func() { + result, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should delete config map", func() { + _, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + + expected := t.NewProxyConfigMap() + actual := &corev1.ConfigMap{} + err = t.client.Get(context.Background(), types.NamespacedName{ + Name: expected.Name, + Namespace: expected.Namespace, + }, actual) + Expect(err).To(HaveOccurred()) + Expect(kerrors.IsNotFound(err)).To(BeTrue()) + }) + }) + + Context("when run out-of-cluster", func() { + BeforeEach(func() { + t.EnvNamespace = nil + }) + + It("should return nil", func() { + result, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should not create config map", func() { + _, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + + expected := t.NewProxyConfigMap() + actual := &corev1.ConfigMap{} + err = t.client.Get(context.Background(), types.NamespacedName{ + Name: expected.Name, + Namespace: expected.Namespace, + }, actual) + Expect(err).To(HaveOccurred()) + Expect(kerrors.IsNotFound(err)).To(BeTrue()) + }) + }) + }) + Describe("reconciling a request", func() { BeforeEach(func() { @@ -122,10 +247,11 @@ var _ = Describe("InsightsController", func() { It("should create the APICast config secret", func() { expected := t.NewInsightsProxySecret() actual := &corev1.Secret{} - t.client.Get(context.Background(), types.NamespacedName{ + err := t.client.Get(context.Background(), types.NamespacedName{ Name: expected.Name, Namespace: expected.Namespace, }, actual) + Expect(err).ToNot(HaveOccurred()) Expect(actual.Labels).To(Equal(expected.Labels)) Expect(actual.Annotations).To(Equal(expected.Annotations)) @@ -137,10 +263,11 @@ var _ = Describe("InsightsController", func() { It("should create the proxy deployment", func() { expected := t.NewInsightsProxyDeployment() actual := &appsv1.Deployment{} - t.client.Get(context.Background(), types.NamespacedName{ + err := t.client.Get(context.Background(), types.NamespacedName{ Name: expected.Name, Namespace: expected.Namespace, }, actual) + Expect(err).ToNot(HaveOccurred()) Expect(actual.Labels).To(Equal(expected.Labels)) Expect(actual.Annotations).To(Equal(expected.Annotations)) @@ -169,10 +296,11 @@ var _ = Describe("InsightsController", func() { It("should create the proxy service", func() { expected := t.NewInsightsProxyService() actual := &corev1.Service{} - t.client.Get(context.Background(), types.NamespacedName{ + err := t.client.Get(context.Background(), types.NamespacedName{ Name: expected.Name, Namespace: expected.Namespace, }, actual) + Expect(err).ToNot(HaveOccurred()) Expect(actual.Labels).To(Equal(expected.Labels)) Expect(actual.Annotations).To(Equal(expected.Annotations)) diff --git a/internal/controllers/insights/insights_controller_unit_test.go b/internal/controllers/insights/insights_controller_unit_test.go new file mode 100644 index 000000000..37d865baa --- /dev/null +++ b/internal/controllers/insights/insights_controller_unit_test.go @@ -0,0 +1,171 @@ +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package insights + +import ( + insightstest "github.com/cryostatio/cryostat-operator/internal/controllers/insights/test" + "github.com/cryostatio/cryostat-operator/internal/test" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +type insightsUnitTestInput struct { + client ctrlclient.Client + controller *InsightsReconciler + objs []ctrlclient.Object + *insightstest.TestUtilsConfig + *insightstest.InsightsTestResources +} + +var _ = Describe("InsightsController", func() { + var t *insightsUnitTestInput + + Describe("configuring watches", func() { + + BeforeEach(func() { + t = &insightsUnitTestInput{ + TestUtilsConfig: &insightstest.TestUtilsConfig{ + EnvInsightsEnabled: &[]bool{true}[0], + EnvInsightsBackendDomain: &[]string{"insights.example.com"}[0], + EnvInsightsProxyImageTag: &[]string{"example.com/proxy:latest"}[0], + EnvNamespace: &[]string{"test"}[0], + }, + InsightsTestResources: &insightstest.InsightsTestResources{ + TestResources: &test.TestResources{ + Namespace: "test", + }, + }, + } + t.objs = []ctrlclient.Object{ + t.NewNamespace(), + t.NewGlobalPullSecret(), + t.NewOperatorDeployment(), + } + }) + + JustBeforeEach(func() { + s := test.NewTestScheme() + logger := zap.New() + logf.SetLogger(logger) + + // Set a CreationTimestamp for created objects to match a real API server + // TODO When using envtest instead of fake client, this is probably no longer needed + err := test.SetCreationTimestamp(t.objs...) + Expect(err).ToNot(HaveOccurred()) + t.client = fake.NewClientBuilder().WithScheme(s).WithObjects(t.objs...).Build() + + config := &InsightsReconcilerConfig{ + Client: test.NewClientWithTimestamp(test.NewTestClient(t.client, t.TestResources)), + Scheme: s, + Log: logger, + Namespace: t.Namespace, + OSUtils: insightstest.NewTestOSUtils(t.TestUtilsConfig), + } + t.controller, err = NewInsightsReconciler(config) + Expect(err).ToNot(HaveOccurred()) + }) + + Context("for secrets", func() { + It("should reconcile global pull secret", func() { + result := t.controller.isPullSecretOrProxyConfig(t.NewGlobalPullSecret()) + Expect(result).To(ConsistOf(t.deploymentReconcileRequest())) + }) + It("should reconcile APICast secret", func() { + result := t.controller.isPullSecretOrProxyConfig(t.NewInsightsProxySecret()) + Expect(result).To(ConsistOf(t.deploymentReconcileRequest())) + }) + It("should not reconcile a secret in another namespace", func() { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: t.NewGlobalPullSecret().Name, + Namespace: "other", + }, + } + result := t.controller.isPullSecretOrProxyConfig(secret) + Expect(result).To(BeEmpty()) + }) + }) + + Context("for deployments", func() { + It("should reconcile proxy deployment", func() { + result := t.controller.isProxyDeployment(t.NewInsightsProxyDeployment()) + Expect(result).To(ConsistOf(t.deploymentReconcileRequest())) + }) + It("should not reconcile a deployment in another namespace", func() { + deploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: t.NewInsightsProxyDeployment().Name, + Namespace: "other", + }, + } + result := t.controller.isProxyDeployment(deploy) + Expect(result).To(BeEmpty()) + }) + }) + + Context("for services", func() { + It("should reconcile proxy service", func() { + result := t.controller.isProxyService(t.NewInsightsProxyService()) + Expect(result).To(ConsistOf(t.deploymentReconcileRequest())) + }) + It("should not reconcile a service in another namespace", func() { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: t.NewInsightsProxyService().Name, + Namespace: "other", + }, + } + result := t.controller.isProxyService(svc) + Expect(result).To(BeEmpty()) + }) + }) + }) +}) + +func (t *insightsUnitTestInput) deploymentReconcileRequest() reconcile.Request { + return reconcile.Request{NamespacedName: types.NamespacedName{Name: "insights-proxy", Namespace: t.Namespace}} +} diff --git a/internal/controllers/insights/setup.go b/internal/controllers/insights/setup.go index 55b493437..48b1082cc 100644 --- a/internal/controllers/insights/setup.go +++ b/internal/controllers/insights/setup.go @@ -38,6 +38,8 @@ package insights import ( "context" + "fmt" + "net/url" "strings" "github.com/cryostatio/cryostat-operator/internal/controllers/common" @@ -52,48 +54,52 @@ import ( ) type InsightsIntegration struct { + Manager ctrl.Manager + Log *logr.Logger common.OSUtils } -func NewInsightsIntegration() *InsightsIntegration { +func NewInsightsIntegration(mgr ctrl.Manager, log *logr.Logger) *InsightsIntegration { return &InsightsIntegration{ + Manager: mgr, + Log: log, OSUtils: &common.DefaultOSUtils{}, } } -func (i *InsightsIntegration) Setup(mgr ctrl.Manager, log logr.Logger) (bool, error) { - enabled := false +func (i *InsightsIntegration) Setup() (*url.URL, error) { + var proxyUrl *url.URL namespace := i.getOperatorNamespace() // This will happen when running the operator locally if len(namespace) == 0 { - log.Info("Operator namespace not detected, disabling Insights integration") - return false, nil + i.Log.Info("Operator namespace not detected, disabling Insights integration") + return nil, nil } ctx := context.Background() if i.isInsightsEnabled() { - err := i.createInsightsController(mgr, namespace, log) + err := i.createInsightsController(namespace) if err != nil { - log.Error(err, "unable to add controller to manager", "controller", "Insights") - return false, err + i.Log.Error(err, "unable to add controller to manager", "controller", "Insights") + return nil, err } // Create a Config Map to be used as a parent of all Insights Proxy related objects - err = i.createConfigMap(ctx, mgr, namespace) + err = i.createConfigMap(ctx, namespace) if err != nil { - log.Error(err, "failed to create config map for Insights") - return false, err + i.Log.Error(err, "failed to create config map for Insights") + return nil, err } - enabled = true + proxyUrl = i.getProxyURL(namespace) } else { // Delete any previously created Config Map (and its children) - err := i.deleteConfigMap(ctx, mgr, namespace) + err := i.deleteConfigMap(ctx, namespace) if err != nil { - log.Error(err, "failed to delete config map for Insights") - return false, err + i.Log.Error(err, "failed to delete config map for Insights") + return nil, err } } - return enabled, nil + return proxyUrl, nil } func (i *InsightsIntegration) isInsightsEnabled() bool { @@ -104,11 +110,11 @@ func (i *InsightsIntegration) getOperatorNamespace() string { return i.GetEnv("NAMESPACE") } -func (i *InsightsIntegration) createInsightsController(mgr ctrl.Manager, namespace string, log logr.Logger) error { +func (i *InsightsIntegration) createInsightsController(namespace string) error { config := &InsightsReconcilerConfig{ - Client: mgr.GetClient(), + Client: i.Manager.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("Insights"), - Scheme: mgr.GetScheme(), + Scheme: i.Manager.GetScheme(), Namespace: namespace, OSUtils: i.OSUtils, } @@ -116,16 +122,16 @@ func (i *InsightsIntegration) createInsightsController(mgr ctrl.Manager, namespa if err != nil { return err } - if err := controller.SetupWithManager(mgr); err != nil { + if err := controller.SetupWithManager(i.Manager); err != nil { return err } return nil } -func (i *InsightsIntegration) createConfigMap(ctx context.Context, mgr ctrl.Manager, namespace string) error { +func (i *InsightsIntegration) createConfigMap(ctx context.Context, namespace string) error { // The config map should be owned by the operator deployment to ensure it and its descendants are garbage collected owner := &appsv1.Deployment{} - err := mgr.GetAPIReader().Get(ctx, types.NamespacedName{Name: OperatorDeploymentName, Namespace: namespace}, owner) + err := i.Manager.GetAPIReader().Get(ctx, types.NamespacedName{Name: OperatorDeploymentName, Namespace: namespace}, owner) if err != nil { return err } @@ -136,17 +142,20 @@ func (i *InsightsIntegration) createConfigMap(ctx context.Context, mgr ctrl.Mana Namespace: namespace, }, } - err = controllerutil.SetControllerReference(owner, cm, mgr.GetScheme()) + err = controllerutil.SetControllerReference(owner, cm, i.Manager.GetScheme()) if err != nil { return err } - err = mgr.GetClient().Create(ctx, cm, &client.CreateOptions{}) + err = i.Manager.GetClient().Create(ctx, cm, &client.CreateOptions{}) + if err == nil { + i.Log.Info("Config Map for Insights created", "name", cm.Name, "namespace", cm.Namespace) + } // This may already exist if the pod restarted return client.IgnoreAlreadyExists(err) } -func (i *InsightsIntegration) deleteConfigMap(ctx context.Context, mgr ctrl.Manager, namespace string) error { +func (i *InsightsIntegration) deleteConfigMap(ctx context.Context, namespace string) error { // Children will be garbage collected cm := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -155,7 +164,17 @@ func (i *InsightsIntegration) deleteConfigMap(ctx context.Context, mgr ctrl.Mana }, } - err := mgr.GetClient().Delete(ctx, cm, &client.DeleteOptions{}) + err := i.Manager.GetClient().Delete(ctx, cm, &client.DeleteOptions{}) + if err == nil { + i.Log.Info("Config Map for Insights deleted", "name", cm.Name, "namespace", cm.Namespace) + } // This may not exist if no config map was previously created return client.IgnoreNotFound(err) } + +func (i *InsightsIntegration) getProxyURL(namespace string) *url.URL { + return &url.URL{ + Scheme: "http", // TODO add https support (r.IsCertManagerInstalled) + Host: fmt.Sprintf("%s.%s.svc.cluster.local", ProxyServiceName, namespace), + } +} diff --git a/internal/controllers/insights/test/manager.go b/internal/controllers/insights/test/manager.go new file mode 100644 index 000000000..b5a2770fa --- /dev/null +++ b/internal/controllers/insights/test/manager.go @@ -0,0 +1,90 @@ +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package test + +import ( + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/config/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/manager" +) + +type FakeManager struct { + ctrl.Manager + client client.Client + scheme *runtime.Scheme + logger *logr.Logger +} + +func NewFakeManager(client client.Client, scheme *runtime.Scheme, logger *logr.Logger) *FakeManager { + return &FakeManager{ + client: client, + scheme: scheme, + logger: logger, + } +} + +func (m *FakeManager) GetClient() client.Client { + return m.client +} + +func (m *FakeManager) GetScheme() *runtime.Scheme { + return m.scheme +} + +func (m *FakeManager) GetAPIReader() client.Reader { + // May need to change if not using a fake client + return m.client +} + +func (m *FakeManager) GetControllerOptions() v1alpha1.ControllerConfigurationSpec { + return v1alpha1.ControllerConfigurationSpec{} +} + +func (m *FakeManager) GetLogger() logr.Logger { + return *m.logger +} + +func (m *FakeManager) SetFields(interface{}) error { + return nil +} + +func (m *FakeManager) Add(manager.Runnable) error { + return nil +} diff --git a/internal/controllers/insights/test/utils.go b/internal/controllers/insights/test/utils.go index be6767a7d..d55e8a048 100644 --- a/internal/controllers/insights/test/utils.go +++ b/internal/controllers/insights/test/utils.go @@ -46,6 +46,7 @@ type TestUtilsConfig struct { EnvInsightsProxyImageTag *string EnvInsightsBackendDomain *string EnvInsightsProxyDomain *string + EnvNamespace *string } type testOSUtils struct { @@ -55,7 +56,7 @@ type testOSUtils struct { func NewTestOSUtils(config *TestUtilsConfig) *testOSUtils { envs := map[string]string{} if config.EnvInsightsEnabled != nil { - envs["DISABLE_SERVICE_TLS"] = strconv.FormatBool(*config.EnvInsightsEnabled) + envs["INSIGHTS_ENABLED"] = strconv.FormatBool(*config.EnvInsightsEnabled) } if config.EnvInsightsProxyImageTag != nil { envs["RELATED_IMAGE_INSIGHTS_PROXY"] = *config.EnvInsightsProxyImageTag @@ -66,6 +67,9 @@ func NewTestOSUtils(config *TestUtilsConfig) *testOSUtils { if config.EnvInsightsProxyDomain != nil { envs["INSIGHTS_PROXY_DOMAIN"] = *config.EnvInsightsProxyDomain } + if config.EnvNamespace != nil { + envs["NAMESPACE"] = *config.EnvNamespace + } return &testOSUtils{envs: envs} } diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 7f636fbeb..fc7c8fcda 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -40,6 +40,7 @@ import ( "context" "errors" "fmt" + "net/url" "regexp" "strconv" "time" @@ -83,7 +84,7 @@ type ReconcilerConfig struct { EventRecorder record.EventRecorder RESTMapper meta.RESTMapper Namespaces []string - Insights bool + InsightsProxy *url.URL // Only defined if Insights is enabled common.ReconcilerTLS } @@ -258,7 +259,9 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance) return reconcile.Result{}, err } - serviceSpecs := &resources.ServiceSpecs{} + serviceSpecs := &resources.ServiceSpecs{ + InsightsURL: r.InsightsProxy, + } err = r.reconcileGrafanaService(ctx, cr, tlsConfig, serviceSpecs) if err != nil { return requeueIfIngressNotReady(reqLogger, err) diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 0f42827b7..f86795a33 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -39,6 +39,7 @@ package controllers_test import ( "context" "fmt" + "net/url" "strconv" "time" @@ -135,6 +136,13 @@ func (t *cryostatTestInput) newReconcilerConfig(scheme *runtime.Scheme, client c logger := zap.New().WithValues("cluster-scoped", t.ClusterScoped) logf.SetLogger(logger) + // Set InsightsURL in config, if provided + var insightsURL *url.URL + if len(t.InsightsURL) > 0 { + url, err := url.Parse(t.InsightsURL) + Expect(err).ToNot(HaveOccurred()) + insightsURL = url + } return &controllers.ReconcilerConfig{ Client: test.NewClientWithTimestamp(test.NewTestClient(client, t.TestResources)), Scheme: scheme, @@ -144,6 +152,7 @@ func (t *cryostatTestInput) newReconcilerConfig(scheme *runtime.Scheme, client c Log: logger, ReconcilerTLS: test.NewTestReconcilerTLS(&t.TestReconcilerConfig), Namespaces: t.watchNamespaces, + InsightsProxy: insightsURL, } } @@ -1297,9 +1306,6 @@ func (c *controllerTest) commonTestsWithoutManager() { It("should add application url to APIServer AdditionalCORSAllowedOrigins", func() { t.expectAPIServer() }) - It("should create an Insights token secret", func() { - t.expectInsightsTokenSecret() - }) It("should add the finalizer", func() { t.expectCryostatFinalizerPresent() }) @@ -1353,6 +1359,17 @@ func (c *controllerTest) commonTestsWithoutManager() { }) }) }) + Context("with Insights enabled", func() { + BeforeEach(func() { + t.InsightsURL = "http://insights-proxy.foo.svc.cluster.local" + }) + JustBeforeEach(func() { + t.reconcileCryostatFully() + }) + It("should create deployment", func() { + t.expectMainDeployment() + }) + }) }) Context("with cert-manager disabled in CR", func() { BeforeEach(func() { @@ -2336,37 +2353,6 @@ func (c *controllerTest) commonTestsWithManager() { }) } }) - - Context("watches the global pull secret", func() { - BeforeEach(func() { - cr := t.NewCryostatWithIngressCertManagerDisabled() - t.objs = append(t.objs, cr.Object) - }) - JustBeforeEach(func() { - // Wait for Insights Token to exist to ensure it will be updated later - Eventually(t.getInsightsTokenValue()).WithTimeout(time.Minute).WithPolling(time.Millisecond).Should(Equal("world")) - // Wait for reconciler to finish - time.Sleep(30 * time.Second) // TODO is there some way to check reconciler queue size? - }) - Context("when the pull secret is updated", func() { - JustBeforeEach(func() { - secret := t.NewGlobalPullSecret() - err := t.Client.Get(context.Background(), types.NamespacedName{Namespace: secret.Namespace, Name: secret.Name}, secret) - Expect(err).ToNot(HaveOccurred()) - - if secret.StringData == nil { - secret.StringData = map[string]string{} - } - secret.StringData[corev1.DockerConfigJsonKey] = `{"auths":{"example.com":{"auth":"hello"},"cloud.openshift.com":{"auth":"foo"}}}` - - err = t.Client.Update(context.Background(), secret) - Expect(err).ToNot(HaveOccurred()) - }) - It("should update the Insights token", func() { - Eventually(t.getInsightsTokenValue()).WithTimeout(time.Minute).WithPolling(time.Millisecond).Should(Equal("foo")) - }) - }) - }) }) } @@ -2657,16 +2643,6 @@ func (t *cryostatTestInput) expectJMXSecret() { Expect(secret.StringData).To(Equal(expectedSecret.StringData)) } -func (t *cryostatTestInput) expectInsightsTokenSecret() { - expectedSecret := t.NewInsightsTokenSecret() - secret := &corev1.Secret{} - err := t.Client.Get(context.Background(), types.NamespacedName{Name: expectedSecret.Name, Namespace: expectedSecret.Namespace}, secret) - Expect(err).ToNot(HaveOccurred()) - - t.checkMetadata(secret, expectedSecret) - Expect(secret.StringData).To(Equal(expectedSecret.StringData)) -} - func (t *cryostatTestInput) expectCoreService() { t.checkService(t.Name, t.NewCryostatService()) } @@ -3198,18 +3174,6 @@ func (t *cryostatTestInput) expectResourcesUnaffected() { } } -func (t *cryostatTestInput) getInsightsTokenValue() func() string { - tokenSecret := t.NewInsightsTokenSecret() - return func() string { - err := t.Client.Get(context.Background(), types.NamespacedName{Namespace: tokenSecret.Namespace, Name: tokenSecret.Name}, tokenSecret) - Expect(ctrlclient.IgnoreNotFound(err)).ToNot(HaveOccurred()) - if tokenSecret != nil { - return string(tokenSecret.Data["token"]) - } - return "" - } -} - func getControllerFunc(clusterScoped bool) func(*controllers.ReconcilerConfig) (controllers.CommonReconciler, error) { if clusterScoped { return newClusterCryostatController diff --git a/internal/main.go b/internal/main.go index 600f9916a..d8c12981b 100644 --- a/internal/main.go +++ b/internal/main.go @@ -39,6 +39,7 @@ package main import ( "flag" "fmt" + "net/url" "os" "strings" @@ -165,13 +166,13 @@ func main() { } // Optionally enable Insights integration. Will only be enabled if INSIGHTS_ENABLED is true - insights, err := insights.NewInsightsIntegration().Setup(mgr, setupLog) + insightsURL, err := insights.NewInsightsIntegration(mgr, &setupLog).Setup() if err != nil { setupLog.Error(err, "failed to set up Insights integration") } config := newReconcilerConfig(mgr, "ClusterCryostat", "clustercryostat-controller", openShift, - certManager, namespaces, insights) + certManager, namespaces, insightsURL) clusterController, err := controllers.NewClusterCryostatReconciler(config) if err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterCryostat") @@ -182,7 +183,7 @@ func main() { os.Exit(1) } config = newReconcilerConfig(mgr, "Cryostat", "cryostat-controller", openShift, certManager, - namespaces, insights) + namespaces, insightsURL) controller, err := controllers.NewCryostatReconciler(config) if err != nil { setupLog.Error(err, "unable to create controller", "controller", "Cryostat") @@ -233,7 +234,7 @@ func isCertManagerInstalled(client discovery.DiscoveryInterface) (bool, error) { } func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName string, openShift bool, - certManager bool, namespaces []string, insights bool) *controllers.ReconcilerConfig { + certManager bool, namespaces []string, insightsURL *url.URL) *controllers.ReconcilerConfig { return &controllers.ReconcilerConfig{ Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName(logName), @@ -243,7 +244,7 @@ func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName str EventRecorder: mgr.GetEventRecorderFor(eventRecorderName), RESTMapper: mgr.GetRESTMapper(), Namespaces: namespaces, - Insights: insights, + InsightsProxy: insightsURL, ReconcilerTLS: common.NewReconcilerTLS(&common.ReconcilerTLSConfig{ Client: mgr.GetClient(), }), diff --git a/internal/test/resources.go b/internal/test/resources.go index ad8ada49f..4dc25acc9 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -73,6 +73,7 @@ type TestResources struct { ReportReplicas int32 ClusterScoped bool TargetNamespaces []string + InsightsURL string } func NewTestScheme() *runtime.Scheme { @@ -1360,6 +1361,14 @@ func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, authProps Value: "200", }) } + + if len(r.InsightsURL) > 0 { + envs = append(envs, + corev1.EnvVar{ + Name: "INSIGHTS_PROXY", + Value: r.InsightsURL, + }) + } return envs } From d39eca77dfa9f6e3d5cc91feb7d5103ba652d949 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Mon, 6 Nov 2023 11:09:36 -0500 Subject: [PATCH 09/19] cleanup --- Makefile | 4 +- .../controllers/clustercryostat_controller.go | 14 +- internal/controllers/common/common_utils.go | 17 ++ .../resource_definitions.go | 68 ++------ internal/controllers/constants/constants.go | 1 + internal/controllers/cryostat_controller.go | 3 +- internal/controllers/insights/apicast.go | 14 ++ internal/controllers/insights/insights.go | 15 +- .../insights/insights_controller.go | 16 +- .../insights/insights_controller_test.go | 14 ++ .../insights/insights_controller_unit_test.go | 14 ++ .../insights/insights_suite_test.go | 14 ++ internal/controllers/insights/setup.go | 19 ++- internal/controllers/insights/test/manager.go | 14 ++ .../controllers/insights/test/resources.go | 14 ++ internal/controllers/insights/test/utils.go | 14 ++ internal/controllers/rbac.go | 1 - internal/controllers/reconciler.go | 3 +- internal/controllers/reconciler_test.go | 159 ------------------ internal/controllers/suite_test.go | 18 +- internal/test/resources.go | 48 ------ 21 files changed, 184 insertions(+), 300 deletions(-) diff --git a/Makefile b/Makefile index 65d6c0482..cdec3ecde 100644 --- a/Makefile +++ b/Makefile @@ -150,9 +150,7 @@ test: test-envtest test-scorecard .PHONY: test-envtest test-envtest: generate manifests fmt vet setup-envtest ## Run tests using envtest. ifneq ($(SKIP_TESTS), true) - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" \ - OPENSHIFT_API_MOD_VERSION=$(shell go list -m -f '{{if .Replace}}{{.Replace.Version}}{{else}}{{.Version}}{{end}}' github.com/openshift/api) \ - $(GO_TEST) -v -coverprofile cover.out ./... + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" $(GO_TEST) -v -coverprofile cover.out ./... endif .PHONY: test-scorecard diff --git a/internal/controllers/clustercryostat_controller.go b/internal/controllers/clustercryostat_controller.go index 5d742c175..a79da08ee 100644 --- a/internal/controllers/clustercryostat_controller.go +++ b/internal/controllers/clustercryostat_controller.go @@ -39,8 +39,7 @@ type ClusterCryostatReconciler struct { } func NewClusterCryostatReconciler(config *ReconcilerConfig) (*ClusterCryostatReconciler, error) { - delegate, err := newReconciler(config, &operatorv1beta1.ClusterCryostat{}, - &operatorv1beta1.ClusterCryostatList{}, false) + delegate, err := newReconciler(config, &operatorv1beta1.ClusterCryostat{}, false) if err != nil { return nil, err } @@ -70,17 +69,6 @@ func NewClusterCryostatReconciler(config *ReconcilerConfig) (*ClusterCryostatRec // +kubebuilder:rbac:groups=console.openshift.io,resources=consolelinks,verbs=get;create;list;update;delete // +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=* -// XXX -// Note: You cannot restrict create or deletecollection requests by their -// resource name. For create, this limitation is because the name of the -// new object may not be known at authorization time. If you restrict list -// or watch by resourceName, clients must include a metadata.name field -// selector in their list or watch request that matches the specified -// resourceName in order to be authorized. For example, kubectl get -// configmaps --field-selector=metadata.name=my-configmap -// -// https://kubernetes.io/docs/reference/access-authn-authz/rbac/#referring-to-resources - // Reconcile processes a ClusterCryostat CR and manages a Cryostat installation accordingly func (r *ClusterCryostatReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { reqLogger := r.Log.WithValues("Request.Name", request.Name) diff --git a/internal/controllers/common/common_utils.go b/internal/controllers/common/common_utils.go index ed7952bbe..454d349f3 100644 --- a/internal/controllers/common/common_utils.go +++ b/internal/controllers/common/common_utils.go @@ -23,6 +23,7 @@ import ( "strings" "time" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -91,3 +92,19 @@ func MergeLabelsAndAnnotations(dest *metav1.ObjectMeta, srcLabels, srcAnnotation dest.Annotations[k] = v } } + +// SeccompProfile returns a SeccompProfile for the restricted +// Pod Security Standard that, on OpenShift, is backwards-compatible +// with OpenShift < 4.11. +// TODO Remove once OpenShift < 4.11 support is dropped +func SeccompProfile(openshift bool) *corev1.SeccompProfile { + // For backward-compatibility with OpenShift < 4.11, + // leave the seccompProfile empty. In OpenShift >= 4.11, + // the restricted-v2 SCC will populate it for us. + if openshift { + return nil + } + return &corev1.SeccompProfile{ + Type: corev1.SeccompProfileTypeRuntimeDefault, + } +} diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 2d9de0690..d2872bd4c 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -360,41 +360,26 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima volumes = append(volumes, eventTemplateVolume) } - if openshift { - // Add Auth properties as a volume if specified (on Openshift) - if cr.Spec.AuthProperties != nil { - authResourceVolume := corev1.Volume{ - Name: "auth-properties-" + cr.Spec.AuthProperties.ConfigMapName, - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: cr.Spec.AuthProperties.ConfigMapName, - }, - Items: []corev1.KeyToPath{ - { - Key: cr.Spec.AuthProperties.Filename, - Path: "OpenShiftAuthManager.properties", - Mode: &readOnlyMode, - }, + // Add Auth properties as a volume if specified (on Openshift) + if openshift && cr.Spec.AuthProperties != nil { + authResourceVolume := corev1.Volume{ + Name: "auth-properties-" + cr.Spec.AuthProperties.ConfigMapName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: cr.Spec.AuthProperties.ConfigMapName, + }, + Items: []corev1.KeyToPath{ + { + Key: cr.Spec.AuthProperties.Filename, + Path: "OpenShiftAuthManager.properties", + Mode: &readOnlyMode, }, }, }, - } - volumes = append(volumes, authResourceVolume) - } - - // Add Insights token secret as a volume - insightsVolume := corev1.Volume{ - Name: "insights-token", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: cr.Name + "-insights-token", - Optional: &[]bool{true}[0], - DefaultMode: &readOnlyMode, - }, }, } - volumes = append(volumes, insightsVolume) + volumes = append(volumes, authResourceVolume) } var podSc *corev1.PodSecurityContext @@ -406,7 +391,7 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima // Ensure PV mounts are writable FSGroup: &fsGroup, RunAsNonRoot: &nonRoot, - SeccompProfile: seccompProfile(openshift), + SeccompProfile: common.SeccompProfile(openshift), } } @@ -549,7 +534,7 @@ func NewPodForReports(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLS nonRoot := true podSc = &corev1.PodSecurityContext{ RunAsNonRoot: &nonRoot, - SeccompProfile: seccompProfile(openshift), + SeccompProfile: common.SeccompProfile(openshift), } } @@ -831,13 +816,6 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag Value: cr.Spec.AuthProperties.ClusterRoleName, }) } - - // Mount Insights token - mounts = append(mounts, corev1.VolumeMount{ - Name: "insights-token", - MountPath: "/var/run/secrets/operator.cryostat.io/insights-token", - ReadOnly: true, - }) } disableBuiltInDiscovery := cr.Spec.TargetDiscoveryOptions != nil && cr.Spec.TargetDiscoveryOptions.BuiltInDiscoveryDisabled @@ -1225,18 +1203,6 @@ func newVolumeForCR(cr *model.CryostatInstance) []corev1.Volume { } } -func seccompProfile(openshift bool) *corev1.SeccompProfile { - // For backward-compatibility with OpenShift < 4.11, - // leave the seccompProfile empty. In OpenShift >= 4.11, - // the restricted-v2 SCC will populate it for us. - if openshift { - return nil - } - return &corev1.SeccompProfile{ - Type: corev1.SeccompProfileTypeRuntimeDefault, - } -} - func useEmptyDir(cr *model.CryostatInstance) bool { return cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.EmptyDir != nil && cr.Spec.StorageOptions.EmptyDir.Enabled diff --git a/internal/controllers/constants/constants.go b/internal/controllers/constants/constants.go index f872ae2e9..cc9541bf3 100644 --- a/internal/controllers/constants/constants.go +++ b/internal/controllers/constants/constants.go @@ -27,6 +27,7 @@ const ( ReportsContainerPort int32 = 10000 LoopbackAddress string = "127.0.0.1" OperatorNamePrefix string = "cryostat-operator-" + OperatorDeploymentName string = "cryostat-operator-controller-manager" HttpPortName string = "http" // CAKey is the key for a CA certificate within a TLS secret CAKey = certMeta.TLSCAKey diff --git a/internal/controllers/cryostat_controller.go b/internal/controllers/cryostat_controller.go index 9ceae7c72..554f16e2d 100644 --- a/internal/controllers/cryostat_controller.go +++ b/internal/controllers/cryostat_controller.go @@ -36,8 +36,7 @@ type CryostatReconciler struct { } func NewCryostatReconciler(config *ReconcilerConfig) (*CryostatReconciler, error) { - delegate, err := newReconciler(config, &operatorv1beta1.Cryostat{}, - &operatorv1beta1.CryostatList{}, true) + delegate, err := newReconciler(config, &operatorv1beta1.Cryostat{}, true) if err != nil { return nil, err } diff --git a/internal/controllers/insights/apicast.go b/internal/controllers/insights/apicast.go index d600805c9..625e9012c 100644 --- a/internal/controllers/insights/apicast.go +++ b/internal/controllers/insights/apicast.go @@ -1,3 +1,17 @@ +// Copyright The Cryostat Authors. +// +// 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. + // Copyright The Cryostat Authors // // The Universal Permissive License (UPL), Version 1.0 diff --git a/internal/controllers/insights/insights.go b/internal/controllers/insights/insights.go index 6b98b6756..214eec663 100644 --- a/internal/controllers/insights/insights.go +++ b/internal/controllers/insights/insights.go @@ -1,3 +1,17 @@ +// Copyright The Cryostat Authors. +// +// 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. + // Copyright The Cryostat Authors // // The Universal Permissive License (UPL), Version 1.0 @@ -146,7 +160,6 @@ func (r *InsightsReconciler) getTokenFromPullSecret(ctx context.Context) (*strin return nil, fmt.Errorf("no %s key present in pull secret", corev1.DockerConfigJsonKey) } - fmt.Println("dockerconfig: " + string(dockerConfigRaw)) // Unmarshal the .dockerconfigjson into a struct dockerConfig := struct { Auths map[string]struct { diff --git a/internal/controllers/insights/insights_controller.go b/internal/controllers/insights/insights_controller.go index c243a7ab5..a4eb7b9c0 100644 --- a/internal/controllers/insights/insights_controller.go +++ b/internal/controllers/insights/insights_controller.go @@ -1,3 +1,17 @@ +// Copyright The Cryostat Authors. +// +// 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. + // Copyright The Cryostat Authors // // The Universal Permissive License (UPL), Version 1.0 @@ -72,9 +86,7 @@ type InsightsReconcilerConfig struct { } const ( - // TODO move to constants, share InsightsConfigMapName = "insights-proxy" - OperatorDeploymentName = "cryostat-operator-controller-manager" ProxyDeploymentName = InsightsConfigMapName ProxyServiceName = ProxyDeploymentName ProxySecretName = "apicastconf" diff --git a/internal/controllers/insights/insights_controller_test.go b/internal/controllers/insights/insights_controller_test.go index 9c97df444..dc22712bf 100644 --- a/internal/controllers/insights/insights_controller_test.go +++ b/internal/controllers/insights/insights_controller_test.go @@ -1,3 +1,17 @@ +// Copyright The Cryostat Authors. +// +// 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. + // Copyright The Cryostat Authors // // The Universal Permissive License (UPL), Version 1.0 diff --git a/internal/controllers/insights/insights_controller_unit_test.go b/internal/controllers/insights/insights_controller_unit_test.go index 37d865baa..d27848008 100644 --- a/internal/controllers/insights/insights_controller_unit_test.go +++ b/internal/controllers/insights/insights_controller_unit_test.go @@ -1,3 +1,17 @@ +// Copyright The Cryostat Authors. +// +// 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. + // Copyright The Cryostat Authors // // The Universal Permissive License (UPL), Version 1.0 diff --git a/internal/controllers/insights/insights_suite_test.go b/internal/controllers/insights/insights_suite_test.go index a9f2fa8a1..e47d78102 100644 --- a/internal/controllers/insights/insights_suite_test.go +++ b/internal/controllers/insights/insights_suite_test.go @@ -1,3 +1,17 @@ +// Copyright The Cryostat Authors. +// +// 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. + // Copyright The Cryostat Authors // // The Universal Permissive License (UPL), Version 1.0 diff --git a/internal/controllers/insights/setup.go b/internal/controllers/insights/setup.go index 48b1082cc..e4783efde 100644 --- a/internal/controllers/insights/setup.go +++ b/internal/controllers/insights/setup.go @@ -1,3 +1,17 @@ +// Copyright The Cryostat Authors. +// +// 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. + // Copyright The Cryostat Authors // // The Universal Permissive License (UPL), Version 1.0 @@ -43,6 +57,7 @@ import ( "strings" "github.com/cryostatio/cryostat-operator/internal/controllers/common" + "github.com/cryostatio/cryostat-operator/internal/controllers/constants" "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -131,7 +146,9 @@ func (i *InsightsIntegration) createInsightsController(namespace string) error { func (i *InsightsIntegration) createConfigMap(ctx context.Context, namespace string) error { // The config map should be owned by the operator deployment to ensure it and its descendants are garbage collected owner := &appsv1.Deployment{} - err := i.Manager.GetAPIReader().Get(ctx, types.NamespacedName{Name: OperatorDeploymentName, Namespace: namespace}, owner) + // Use the APIReader instead of the cache, since the cache may not be synced yet + err := i.Manager.GetAPIReader().Get(ctx, types.NamespacedName{ + Name: constants.OperatorDeploymentName, Namespace: namespace}, owner) if err != nil { return err } diff --git a/internal/controllers/insights/test/manager.go b/internal/controllers/insights/test/manager.go index b5a2770fa..c34641c24 100644 --- a/internal/controllers/insights/test/manager.go +++ b/internal/controllers/insights/test/manager.go @@ -1,3 +1,17 @@ +// Copyright The Cryostat Authors. +// +// 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. + // Copyright The Cryostat Authors // // The Universal Permissive License (UPL), Version 1.0 diff --git a/internal/controllers/insights/test/resources.go b/internal/controllers/insights/test/resources.go index 20349bc52..500094f90 100644 --- a/internal/controllers/insights/test/resources.go +++ b/internal/controllers/insights/test/resources.go @@ -1,3 +1,17 @@ +// Copyright The Cryostat Authors. +// +// 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. + // Copyright The Cryostat Authors // // The Universal Permissive License (UPL), Version 1.0 diff --git a/internal/controllers/insights/test/utils.go b/internal/controllers/insights/test/utils.go index d55e8a048..b8df553f1 100644 --- a/internal/controllers/insights/test/utils.go +++ b/internal/controllers/insights/test/utils.go @@ -1,3 +1,17 @@ +// Copyright The Cryostat Authors. +// +// 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. + // Copyright The Cryostat Authors // // The Universal Permissive License (UPL), Version 1.0 diff --git a/internal/controllers/rbac.go b/internal/controllers/rbac.go index 6c31391f3..92ab4eab6 100644 --- a/internal/controllers/rbac.go +++ b/internal/controllers/rbac.go @@ -151,7 +151,6 @@ func (r *Reconciler) reconcileRoleBinding(ctx context.Context, cr *model.Cryosta } func (r *Reconciler) newClusterRoleBinding(cr *model.CryostatInstance) *rbacv1.ClusterRoleBinding { - fmt.Println("CRB KIND: " + r.gvk.Kind) // XXX return &rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: common.ClusterUniqueName(r.gvk, cr.Name, cr.InstallNamespace), diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 532d2720a..0980e919c 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -125,8 +125,7 @@ var reportsDeploymentConditions = deploymentConditionTypeMap{ operatorv1beta1.ConditionTypeReportsDeploymentReplicaFailure: appsv1.DeploymentReplicaFailure, } -func newReconciler(config *ReconcilerConfig, objType client.Object, listType client.ObjectList, - isNamespaced bool) (*Reconciler, error) { +func newReconciler(config *ReconcilerConfig, objType client.Object, isNamespaced bool) (*Reconciler, error) { gvk, err := apiutil.GVKForObject(objType, config.Scheme) if err != nil { return nil, err diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 2c3726e47..7b12b2752 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -18,7 +18,6 @@ import ( "context" "fmt" "net/url" - "strconv" "time" certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" @@ -39,9 +38,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" - ctrl "sigs.k8s.io/controller-runtime" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -85,7 +82,6 @@ func (c *controllerTest) commonBeforeEach() *cryostatTestInput { t.objs = []ctrlclient.Object{ t.NewNamespace(), t.NewApiServer(), - t.NewGlobalPullSecret(), // TODO OpenShift only? } return t } @@ -104,7 +100,6 @@ func (c *controllerTest) commonJustBeforeEach(t *cryostatTestInput) { func (c *controllerTest) commonJustAfterEach(t *cryostatTestInput) { for _, obj := range t.objs { - fmt.Printf("Deleting: %v\n", obj) // XXX err := ctrlclient.IgnoreNotFound(t.Client.Delete(context.Background(), obj)) Expect(err).ToNot(HaveOccurred()) } @@ -220,11 +215,6 @@ func expectSuccessful(t **cryostatTestInput) { } func (c *controllerTest) commonTests() { - c.commonTestsWithoutManager() - //c.commonTestsWithManager() -} - -func (c *controllerTest) commonTestsWithoutManager() { var t *cryostatTestInput Describe("reconciling a request in OpenShift", func() { @@ -1311,7 +1301,6 @@ func (c *controllerTest) commonTestsWithoutManager() { BeforeEach(func() { t.objs = []ctrlclient.Object{ t.NewCryostat().Object, t.NewNamespaceWithSCCSupGroups(), t.NewApiServer(), - t.NewGlobalPullSecret(), } }) It("should set fsGroup to value derived from namespace", func() { @@ -2220,154 +2209,6 @@ func (c *controllerTest) commonTestsWithoutManager() { }) } -func (c *controllerTest) commonTestsWithManager() { - var t *cryostatTestInput - - Describe("setting up the controller", func() { - var done chan interface{} - var cancel context.CancelFunc - - BeforeEach(func() { - t = &cryostatTestInput{ - TestReconcilerConfig: test.TestReconcilerConfig{ - GeneratedPasswords: []string{"grafana", "credentials_database", "jmx", "keystore"}, - }, - TestResources: &test.TestResources{ - Name: "cryostat", - Namespace: "test", - TLS: true, - ExternalTLS: true, - OpenShift: true, - ClusterScoped: c.clusterScoped, - }, - } - suffix := strconv.Itoa(nameSuffix) - t.Name += "-" + suffix - t.Namespace += "-" + suffix - t.TargetNamespaces = []string{t.Namespace} - t.watchNamespaces = []string{t.Namespace} - - t.TLS = false // TODO can use InstallCertManager - t.objs = []ctrlclient.Object{ - t.NewNamespace(), - t.NewApiServer(), - t.NewGlobalPullSecret(), - } - }) - JustBeforeEach(func() { - k8sClient, err := ctrlclient.New(cfg, ctrlclient.Options{Scheme: scheme.Scheme}) - Expect(err).NotTo(HaveOccurred()) - Expect(k8sClient).NotTo(BeNil()) - - // Create openshift-config namespace first - // Don't include this with other objects, since those will be deleted - // after the test. EnvTest doesn't support namespace deletion, so an - // attempt to delete openshift-config will prevent us from using it - // for subsequent tests. - // See: https://book.kubebuilder.io/reference/envtest.html#namespace-usage-limitation - ns := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "openshift-config", - }, - } - err = k8sClient.Create(context.Background(), ns) - Expect(ctrlclient.IgnoreAlreadyExists(err)).ToNot(HaveOccurred()) - - for _, obj := range t.objs { - fmt.Printf("Creating: %v\n", obj) // XXX - err := k8sClient.Create(context.TODO(), obj) - Expect(err).ToNot(HaveOccurred()) - } - - t.Client = k8sClient - controller, err := c.constructorFunc(t.newReconcilerConfig(scheme.Scheme, t.Client)) - Expect(err).ToNot(HaveOccurred()) - t.controller = controller - manager, err := ctrl.NewManager(cfg, ctrl.Options{ - Scheme: test.NewTestScheme(), - MetricsBindAddress: "0", - Port: 9443, - HealthProbeBindAddress: "0", - LeaderElection: false, - }) - Expect(err).ToNot(HaveOccurred()) - err = t.controller.SetupWithManager(manager) - Expect(err).ToNot(HaveOccurred()) - - ctx, fn := context.WithCancel(context.Background()) - cancel = fn - done = make(chan interface{}) - go func() { - defer GinkgoRecover() - err := manager.Start(ctx) - Expect(err).ToNot(HaveOccurred()) - fmt.Println("Stopping Manager") // XXX - close(done) - }() - }) - JustAfterEach(func() { - for _, obj := range t.objs { - fmt.Printf("Deleting: %v\n", obj) // XXX - err := ctrlclient.IgnoreNotFound(t.Client.Delete(context.Background(), obj)) - Expect(err).ToNot(HaveOccurred()) - } - }) - AfterEach(func() { - if cancel != nil { - cancel() - <-done - } - nameSuffix++ - }) - - Context("watches the configured namespace(s)", func() { - var otherNamespace string - - BeforeEach(func() { - suffix := strconv.Itoa(nameSuffix) - saveNS := t.Namespace - otherNamespace = "other-" + suffix - t.Namespace = otherNamespace - otherNS := t.NewNamespace() - t.Namespace = saveNS - t.objs = append(t.objs, otherNS) - }) - Context("creating a CR in the watched namespace", func() { - BeforeEach(func() { - t.objs = append(t.objs, t.NewCryostatWithIngressCertManagerDisabled().Object) - }) - It("should reconcile the CR", func() { - Eventually(func() bool { - cr := t.getCryostatInstance() - if cr != nil && cr.Status != nil && len(cr.Status.ApplicationURL) > 0 { - return true - } - return false - }).WithTimeout(time.Minute).WithPolling(time.Millisecond).Should(BeTrue()) - }) - }) - if !c.clusterScoped { - Context("creating a CR in a different namespace", func() { - BeforeEach(func() { - t.Namespace = otherNamespace - t.TargetNamespaces = []string{otherNamespace} - t.objs = append(t.objs, t.NewCryostatWithIngressCertManagerDisabled().Object) - }) - It("should not reconcile the CR", func() { - Consistently(func() bool { - cr := t.getCryostatInstance() - if cr != nil && cr.Status != nil && len(cr.Status.ApplicationURL) > 0 { - return true - } - return false - }).WithTimeout(time.Minute).WithPolling(time.Millisecond).Should(BeFalse()) - }) - }) - } - }) - }) -} - func (t *cryostatTestInput) expectRoutes() { if !t.Minimal { t.checkRoute(t.NewGrafanaRoute()) diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index f22376a65..36391d97c 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -15,9 +15,6 @@ package controllers_test import ( - "fmt" - "go/build" - "os" "path/filepath" "testing" @@ -45,7 +42,6 @@ import ( var cfg *rest.Config var k8sClient client.Client var testEnv *envtest.Environment -var nameSuffix int func TestAPIs(t *testing.T) { RegisterFailHandler(Fail) @@ -57,20 +53,9 @@ var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) By("bootstrapping test environment") - openshiftModVersion := os.Getenv("OPENSHIFT_API_MOD_VERSION") - Expect(openshiftModVersion).ToNot(BeEmpty(), "OPENSHIFT_API_MOD_VERSION environment variable must be set") - openshiftPrefix := []string{build.Default.GOPATH, "pkg", "mod", "github.com", "openshift", - "api@" + openshiftModVersion} testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{ - filepath.Join("..", "..", "config", "crd", "bases"), - filepath.Join(append(openshiftPrefix, "route", "v1")...), - filepath.Join(append(openshiftPrefix, "console", "v1")...), - filepath.Join(append(openshiftPrefix, "config", "v1")...), - }, - ErrorIfCRDPathMissing: true, + CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")}, } - fmt.Println(testEnv.CRDDirectoryPaths) var err error // cfg is defined in this file globally. @@ -99,7 +84,6 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient).NotTo(BeNil()) - nameSuffix = 0 }) var _ = AfterSuite(func() { diff --git a/internal/test/resources.go b/internal/test/resources.go index 3f0f805b6..79c957263 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1665,14 +1665,6 @@ func (r *TestResources) NewCoreVolumeMounts() []corev1.VolumeMount { MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-tls", r.Name), }) } - if r.OpenShift { - mounts = append(mounts, - corev1.VolumeMount{ - Name: "insights-token", - ReadOnly: true, - MountPath: "/var/run/secrets/operator.cryostat.io/insights-token", - }) - } return mounts } @@ -2036,20 +2028,6 @@ func (r *TestResources) newVolumes(certProjections []corev1.VolumeProjection) [] }, }) - if r.OpenShift { - volumes = append(volumes, - corev1.Volume{ - Name: "insights-token", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: r.Name + "-insights-token", - Optional: &[]bool{true}[0], - DefaultMode: &readOnlymode, - }, - }, - }) - } - return volumes } @@ -2861,32 +2839,6 @@ func (r *TestResources) NewLockConfigMap() *corev1.ConfigMap { } } -func (r *TestResources) NewGlobalPullSecret() *corev1.Secret { - config := `{"auths":{"example.com":{"auth":"hello"},"cloud.openshift.com":{"auth":"world"}}}` - return &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pull-secret", - Namespace: "openshift-config", - }, - Data: map[string][]byte{ - corev1.DockerConfigJsonKey: []byte(config), - }, - } -} - -func (r *TestResources) NewInsightsTokenSecret() *corev1.Secret { - config := "world" - return &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: r.Name + "-insights-token", - Namespace: r.Namespace, - }, - StringData: map[string]string{ - "token": config, - }, - } -} - func (r *TestResources) getClusterUniqueName() string { prefix := "cryostat-" if r.ClusterScoped { From 1c6b74cedae4f9ede025f70dec883ed05b18920e Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Mon, 6 Nov 2023 11:53:29 -0500 Subject: [PATCH 10/19] Convert filter test to unit test --- .../controllers/cryostat_controller_test.go | 44 ----------- internal/controllers/reconciler.go | 4 +- internal/controllers/reconciler_test.go | 1 + internal/controllers/reconciler_unit_test.go | 79 +++++++++++++++++++ 4 files changed, 82 insertions(+), 46 deletions(-) create mode 100644 internal/controllers/reconciler_unit_test.go diff --git a/internal/controllers/cryostat_controller_test.go b/internal/controllers/cryostat_controller_test.go index 48b5a3eb6..2530f4edd 100644 --- a/internal/controllers/cryostat_controller_test.go +++ b/internal/controllers/cryostat_controller_test.go @@ -16,11 +16,7 @@ package controllers_test import ( "github.com/cryostatio/cryostat-operator/internal/controllers" - "github.com/cryostatio/cryostat-operator/internal/controllers/model" . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/predicate" ) var _ = Describe("CryostatController", func() { @@ -30,46 +26,6 @@ var _ = Describe("CryostatController", func() { } c.commonTests() - - Describe("filtering requests", func() { - Context("watches the configured namespace(s)", func() { - var t *cryostatTestInput - var filter predicate.Predicate - var cr *model.CryostatInstance - - BeforeEach(func() { - t = c.commonBeforeEach() - t.watchNamespaces = []string{t.Namespace} - }) - JustBeforeEach(func() { - c.commonJustBeforeEach(t) - filter = controllers.NamespaceEventFilter(t.Client.Scheme(), t.watchNamespaces) - }) - Context("creating a CR in the watched namespace", func() { - BeforeEach(func() { - cr = t.NewCryostat() - }) - It("should reconcile the CR", func() { - result := filter.Create(event.CreateEvent{ - Object: cr.Object, - }) - Expect(result).To(BeTrue()) - }) - }) - Context("creating a CR in a non-watched namespace", func() { - BeforeEach(func() { - t.Namespace = "something-else" - cr = t.NewCryostat() - }) - It("should reconcile the CR", func() { - result := filter.Create(event.CreateEvent{ - Object: cr.Object, - }) - Expect(result).To(BeFalse()) - }) - }) - }) - }) }) func newCryostatController(config *controllers.ReconcilerConfig) (controllers.CommonReconciler, error) { diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 0980e919c..61cc8f1f2 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -292,7 +292,7 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance) return reconcile.Result{}, nil } -func NamespaceEventFilter(scheme *runtime.Scheme, namespaceList []string) predicate.Predicate { +func namespaceEventFilter(scheme *runtime.Scheme, namespaceList []string) predicate.Predicate { namespaces := namespacesToSet(namespaceList) return predicate.NewPredicateFuncs(func(object client.Object) bool { // Restrict watch for namespaced objects to specified namespaces @@ -312,7 +312,7 @@ func (r *Reconciler) setupWithManager(mgr ctrl.Manager, impl reconcile.Reconcile if r.isNamespaced { // TODO remove this once only AllNamespace mode is supported - c = c.WithEventFilter(NamespaceEventFilter(mgr.GetScheme(), r.Namespaces)) + c = c.WithEventFilter(namespaceEventFilter(mgr.GetScheme(), r.Namespaces)) } // Watch for changes to secondary resources and requeue the owner Cryostat diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 7b12b2752..956b9ccf0 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -83,6 +83,7 @@ func (c *controllerTest) commonBeforeEach() *cryostatTestInput { t.NewNamespace(), t.NewApiServer(), } + t.watchNamespaces = []string{t.Namespace} return t } diff --git a/internal/controllers/reconciler_unit_test.go b/internal/controllers/reconciler_unit_test.go new file mode 100644 index 000000000..bec0e97de --- /dev/null +++ b/internal/controllers/reconciler_unit_test.go @@ -0,0 +1,79 @@ +// Copyright The Cryostat Authors. +// +// 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 controllers + +import ( + "github.com/cryostatio/cryostat-operator/internal/controllers/model" + "github.com/cryostatio/cryostat-operator/internal/test" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +type cryostatUnitTestInput struct { + scheme *runtime.Scheme + watchNamespaces []string + *test.TestResources +} + +var _ = Describe("Reconciler", func() { + Describe("filtering requests", func() { + Context("watches the configured namespace(s)", func() { + var t *cryostatUnitTestInput + var filter predicate.Predicate + var cr *model.CryostatInstance + + BeforeEach(func() { + resources := &test.TestResources{ + Name: "cryostat", + Namespace: "test", + } + t = &cryostatUnitTestInput{ + scheme: test.NewTestScheme(), + watchNamespaces: []string{resources.Namespace}, + TestResources: resources, + } + }) + JustBeforeEach(func() { + filter = namespaceEventFilter(t.scheme, t.watchNamespaces) + }) + Context("creating a CR in the watched namespace", func() { + BeforeEach(func() { + cr = t.NewCryostat() + }) + It("should reconcile the CR", func() { + result := filter.Create(event.CreateEvent{ + Object: cr.Object, + }) + Expect(result).To(BeTrue()) + }) + }) + Context("creating a CR in a non-watched namespace", func() { + BeforeEach(func() { + t.Namespace = "something-else" + cr = t.NewCryostat() + }) + It("should reconcile the CR", func() { + result := filter.Create(event.CreateEvent{ + Object: cr.Object, + }) + Expect(result).To(BeFalse()) + }) + }) + }) + }) +}) From 4cc7c00e173489f66cacf532ba67588ae35b435c Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Mon, 6 Nov 2023 12:02:17 -0500 Subject: [PATCH 11/19] Move setup to its own test file --- internal/controllers/insights/insights.go | 15 +- .../insights/insights_controller_test.go | 126 ----------- internal/controllers/insights/setup_test.go | 198 ++++++++++++++++++ internal/main.go | 2 +- 4 files changed, 200 insertions(+), 141 deletions(-) create mode 100644 internal/controllers/insights/setup_test.go diff --git a/internal/controllers/insights/insights.go b/internal/controllers/insights/insights.go index 214eec663..92ff149a8 100644 --- a/internal/controllers/insights/insights.go +++ b/internal/controllers/insights/insights.go @@ -360,20 +360,7 @@ func (r *InsightsReconciler) getProxyPodSpec() *corev1.PodSpec { }, SecurityContext: &corev1.PodSecurityContext{ RunAsNonRoot: &nonRoot, - SeccompProfile: seccompProfile(true), + SeccompProfile: common.SeccompProfile(true), }, } } - -// TODO dedup or remove -func seccompProfile(openshift bool) *corev1.SeccompProfile { - // For backward-compatibility with OpenShift < 4.11, - // leave the seccompProfile empty. In OpenShift >= 4.11, - // the restricted-v2 SCC will populate it for us. - if openshift { - return nil - } - return &corev1.SeccompProfile{ - Type: corev1.SeccompProfileTypeRuntimeDefault, - } -} diff --git a/internal/controllers/insights/insights_controller_test.go b/internal/controllers/insights/insights_controller_test.go index dc22712bf..32c4da1c9 100644 --- a/internal/controllers/insights/insights_controller_test.go +++ b/internal/controllers/insights/insights_controller_test.go @@ -61,7 +61,6 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -82,132 +81,7 @@ type insightsTestInput struct { var _ = Describe("InsightsController", func() { var t *insightsTestInput - Describe("setting up", func() { - var integration *insights.InsightsIntegration - - BeforeEach(func() { - t = &insightsTestInput{ - TestUtilsConfig: &insightstest.TestUtilsConfig{ - EnvInsightsEnabled: &[]bool{true}[0], - EnvInsightsBackendDomain: &[]string{"insights.example.com"}[0], - EnvInsightsProxyImageTag: &[]string{"example.com/proxy:latest"}[0], - EnvNamespace: &[]string{"test"}[0], - }, - InsightsTestResources: &insightstest.InsightsTestResources{ - TestResources: &test.TestResources{ - Namespace: "test", - }, - }, - } - t.objs = []ctrlclient.Object{ - t.NewNamespace(), - t.NewGlobalPullSecret(), - t.NewOperatorDeployment(), - } - }) - - JustBeforeEach(func() { - s := test.NewTestScheme() - logger := zap.New() - logf.SetLogger(logger) - - // Set a CreationTimestamp for created objects to match a real API server - // TODO When using envtest instead of fake client, this is probably no longer needed - err := test.SetCreationTimestamp(t.objs...) - Expect(err).ToNot(HaveOccurred()) - t.client = fake.NewClientBuilder().WithScheme(s).WithObjects(t.objs...).Build() - - manager := insightstest.NewFakeManager(test.NewClientWithTimestamp(test.NewTestClient(t.client, t.TestResources)), - s, &logger) - integration = insights.NewInsightsIntegration(manager, &logger) - integration.OSUtils = insightstest.NewTestOSUtils(t.TestUtilsConfig) - }) - - Context("with defaults", func() { - It("should return proxy URL", func() { - result, err := integration.Setup() - Expect(err).ToNot(HaveOccurred()) - Expect(result).ToNot(BeNil()) - Expect(result.String()).To(Equal("http://insights-proxy.test.svc.cluster.local")) - }) - - It("should create config map", func() { - _, err := integration.Setup() - Expect(err).ToNot(HaveOccurred()) - - expected := t.NewProxyConfigMap() - actual := &corev1.ConfigMap{} - err = t.client.Get(context.Background(), types.NamespacedName{ - Name: expected.Name, - Namespace: expected.Namespace, - }, actual) - Expect(err).ToNot(HaveOccurred()) - - Expect(actual.Labels).To(Equal(expected.Labels)) - Expect(actual.Annotations).To(Equal(expected.Annotations)) - Expect(metav1.IsControlledBy(actual, t.NewOperatorDeployment())).To(BeTrue()) - Expect(actual.Data).To(BeEmpty()) - }) - }) - - Context("with Insights disabled", func() { - BeforeEach(func() { - t.EnvInsightsEnabled = &[]bool{false}[0] - t.objs = append(t.objs, - t.NewProxyConfigMap(), - ) - }) - - It("should return nil", func() { - result, err := integration.Setup() - Expect(err).ToNot(HaveOccurred()) - Expect(result).To(BeNil()) - }) - - It("should delete config map", func() { - _, err := integration.Setup() - Expect(err).ToNot(HaveOccurred()) - - expected := t.NewProxyConfigMap() - actual := &corev1.ConfigMap{} - err = t.client.Get(context.Background(), types.NamespacedName{ - Name: expected.Name, - Namespace: expected.Namespace, - }, actual) - Expect(err).To(HaveOccurred()) - Expect(kerrors.IsNotFound(err)).To(BeTrue()) - }) - }) - - Context("when run out-of-cluster", func() { - BeforeEach(func() { - t.EnvNamespace = nil - }) - - It("should return nil", func() { - result, err := integration.Setup() - Expect(err).ToNot(HaveOccurred()) - Expect(result).To(BeNil()) - }) - - It("should not create config map", func() { - _, err := integration.Setup() - Expect(err).ToNot(HaveOccurred()) - - expected := t.NewProxyConfigMap() - actual := &corev1.ConfigMap{} - err = t.client.Get(context.Background(), types.NamespacedName{ - Name: expected.Name, - Namespace: expected.Namespace, - }, actual) - Expect(err).To(HaveOccurred()) - Expect(kerrors.IsNotFound(err)).To(BeTrue()) - }) - }) - }) - Describe("reconciling a request", func() { - BeforeEach(func() { t = &insightsTestInput{ TestUtilsConfig: &insightstest.TestUtilsConfig{ diff --git a/internal/controllers/insights/setup_test.go b/internal/controllers/insights/setup_test.go new file mode 100644 index 000000000..7759b0e6b --- /dev/null +++ b/internal/controllers/insights/setup_test.go @@ -0,0 +1,198 @@ +// Copyright The Cryostat Authors. +// +// 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. + +// Copyright The Cryostat Authors +// +// The Universal Permissive License (UPL), Version 1.0 +// +// Subject to the condition set forth below, permission is hereby granted to any +// person obtaining a copy of this software, associated documentation and/or data +// (collectively the "Software"), free of charge and under any and all copyright +// rights in the Software, and any and all patent rights owned or freely +// licensable by each licensor hereunder covering either (i) the unmodified +// Software as contributed to or provided by such licensor, or (ii) the Larger +// Works (as defined below), to deal in both +// +// (a) the Software, and +// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if +// one is included with the Software (each a "Larger Work" to which the Software +// is contributed by such licensors), +// +// without restriction, including without limitation the rights to copy, create +// derivative works of, display, perform, and distribute the Software and make, +// use, sell, offer for sale, import, export, have made, and have sold the +// Software and the Larger Work(s), and to sublicense the foregoing rights on +// either these or other terms. +// +// This license is subject to the following condition: +// The above copyright notice and either this complete permission notice or at +// a minimum a reference to the UPL must be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package insights_test + +import ( + "context" + + "github.com/cryostatio/cryostat-operator/internal/controllers/insights" + insightstest "github.com/cryostatio/cryostat-operator/internal/controllers/insights/test" + "github.com/cryostatio/cryostat-operator/internal/test" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +var _ = Describe("InsightsIntegration", func() { + var t *insightsTestInput + + Describe("setting up", func() { + var integration *insights.InsightsIntegration + + BeforeEach(func() { + t = &insightsTestInput{ + TestUtilsConfig: &insightstest.TestUtilsConfig{ + EnvInsightsEnabled: &[]bool{true}[0], + EnvInsightsBackendDomain: &[]string{"insights.example.com"}[0], + EnvInsightsProxyImageTag: &[]string{"example.com/proxy:latest"}[0], + EnvNamespace: &[]string{"test"}[0], + }, + InsightsTestResources: &insightstest.InsightsTestResources{ + TestResources: &test.TestResources{ + Namespace: "test", + }, + }, + } + t.objs = []ctrlclient.Object{ + t.NewNamespace(), + t.NewGlobalPullSecret(), + t.NewOperatorDeployment(), + } + }) + + JustBeforeEach(func() { + s := test.NewTestScheme() + logger := zap.New() + logf.SetLogger(logger) + + // Set a CreationTimestamp for created objects to match a real API server + // TODO When using envtest instead of fake client, this is probably no longer needed + err := test.SetCreationTimestamp(t.objs...) + Expect(err).ToNot(HaveOccurred()) + t.client = fake.NewClientBuilder().WithScheme(s).WithObjects(t.objs...).Build() + + manager := insightstest.NewFakeManager(test.NewClientWithTimestamp(test.NewTestClient(t.client, t.TestResources)), + s, &logger) + integration = insights.NewInsightsIntegration(manager, &logger) + integration.OSUtils = insightstest.NewTestOSUtils(t.TestUtilsConfig) + }) + + Context("with defaults", func() { + It("should return proxy URL", func() { + result, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + Expect(result).ToNot(BeNil()) + Expect(result.String()).To(Equal("http://insights-proxy.test.svc.cluster.local")) + }) + + It("should create config map", func() { + _, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + + expected := t.NewProxyConfigMap() + actual := &corev1.ConfigMap{} + err = t.client.Get(context.Background(), types.NamespacedName{ + Name: expected.Name, + Namespace: expected.Namespace, + }, actual) + Expect(err).ToNot(HaveOccurred()) + + Expect(actual.Labels).To(Equal(expected.Labels)) + Expect(actual.Annotations).To(Equal(expected.Annotations)) + Expect(metav1.IsControlledBy(actual, t.NewOperatorDeployment())).To(BeTrue()) + Expect(actual.Data).To(BeEmpty()) + }) + }) + + Context("with Insights disabled", func() { + BeforeEach(func() { + t.EnvInsightsEnabled = &[]bool{false}[0] + t.objs = append(t.objs, + t.NewProxyConfigMap(), + ) + }) + + It("should return nil", func() { + result, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should delete config map", func() { + _, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + + expected := t.NewProxyConfigMap() + actual := &corev1.ConfigMap{} + err = t.client.Get(context.Background(), types.NamespacedName{ + Name: expected.Name, + Namespace: expected.Namespace, + }, actual) + Expect(err).To(HaveOccurred()) + Expect(kerrors.IsNotFound(err)).To(BeTrue()) + }) + }) + + Context("when run out-of-cluster", func() { + BeforeEach(func() { + t.EnvNamespace = nil + }) + + It("should return nil", func() { + result, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should not create config map", func() { + _, err := integration.Setup() + Expect(err).ToNot(HaveOccurred()) + + expected := t.NewProxyConfigMap() + actual := &corev1.ConfigMap{} + err = t.client.Get(context.Background(), types.NamespacedName{ + Name: expected.Name, + Namespace: expected.Namespace, + }, actual) + Expect(err).To(HaveOccurred()) + Expect(kerrors.IsNotFound(err)).To(BeTrue()) + }) + }) + }) +}) diff --git a/internal/main.go b/internal/main.go index 2182a2915..155a39d49 100644 --- a/internal/main.go +++ b/internal/main.go @@ -107,7 +107,7 @@ func main() { HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "d696d7ab.redhat.com", - ClientDisableCacheFor: disableCache, // FIXME can probable remove + ClientDisableCacheFor: disableCache, // TODO can probably remove }) if err != nil { setupLog.Error(err, "unable to start manager") From d42d065a0f22943b463bd61a0ec105e4dd7a6c6b Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Mon, 6 Nov 2023 12:08:32 -0500 Subject: [PATCH 12/19] cleanup --- internal/controllers/insights/insights_controller.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/controllers/insights/insights_controller.go b/internal/controllers/insights/insights_controller.go index a4eb7b9c0..7ec275b0c 100644 --- a/internal/controllers/insights/insights_controller.go +++ b/internal/controllers/insights/insights_controller.go @@ -69,7 +69,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" ) -// TODO +// InsightsReconciler reconciles the Insights proxy for Cryostat agents type InsightsReconciler struct { *InsightsReconcilerConfig backendDomain string @@ -77,6 +77,7 @@ type InsightsReconciler struct { proxyImageTag string } +// InsightsReconcilerConfig contains configuration to create an InsightsReconciler type InsightsReconcilerConfig struct { client.Client Log logr.Logger @@ -97,6 +98,7 @@ const ( EnvInsightsProxyImageTag = "RELATED_IMAGE_INSIGHTS_PROXY" ) +// NewInsightsReconciler creates an InsightsReconciler using the provided configuration func NewInsightsReconciler(config *InsightsReconcilerConfig) (*InsightsReconciler, error) { backendDomain := config.GetEnv(EnvInsightsBackendDomain) if len(backendDomain) == 0 { @@ -119,7 +121,7 @@ func NewInsightsReconciler(config *InsightsReconcilerConfig) (*InsightsReconcile // +kubebuilder:rbac:groups=apps,resources=deployments;deployments/finalizers,verbs=* // +kubebuilder:rbac:groups="",resources=services;secrets;configmaps;configmaps/finalizers,verbs=* -// Reconcile processes a Cryostat CR and manages a Cryostat installation accordingly +// Reconcile processes the Insights proxy deployment and configures it accordingly func (r *InsightsReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { reqLogger := r.Log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name) reqLogger.Info("Reconciling Insights Proxy") @@ -136,6 +138,7 @@ func (r *InsightsReconciler) Reconcile(ctx context.Context, request ctrl.Request func (r *InsightsReconciler) SetupWithManager(mgr ctrl.Manager) error { c := ctrl.NewControllerManagedBy(mgr). Named("insights"). + // Filter controller to watch only specific objects we care about Watches(&source.Kind{Type: &corev1.Secret{}}, handler.EnqueueRequestsFromMapFunc(r.isPullSecretOrProxyConfig)). Watches(&source.Kind{Type: &appsv1.Deployment{}}, From e4d6fc2eaa3b56b1db623389f4300dd9325d4e56 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Mon, 6 Nov 2023 14:32:24 -0500 Subject: [PATCH 13/19] Add resource requirements and more tests --- internal/controllers/insights/insights.go | 182 ++++++++++-------- .../insights/insights_controller_test.go | 89 ++++++++- .../controllers/insights/test/resources.go | 87 ++++++++- internal/controllers/reconciler_test.go | 45 +---- internal/test/expect.go | 57 ++++++ 5 files changed, 339 insertions(+), 121 deletions(-) create mode 100644 internal/test/expect.go diff --git a/internal/controllers/insights/insights.go b/internal/controllers/insights/insights.go index 92ff149a8..c0d9f4d85 100644 --- a/internal/controllers/insights/insights.go +++ b/internal/controllers/insights/insights.go @@ -61,6 +61,7 @@ import ( "github.com/cryostatio/cryostat-operator/internal/controllers/constants" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" @@ -225,7 +226,7 @@ func (r *InsightsReconciler) createOrUpdateProxyDeployment(ctx context.Context, } // Update pod template spec - deploy.Spec.Template.Spec = *r.getProxyPodSpec() + r.createOrUpdateProxyPodSpec(deploy) // Update pod template metadata common.MergeLabelsAndAnnotations(&deploy.Spec.Template.ObjectMeta, labels, annotations) return nil @@ -274,93 +275,118 @@ func (r *InsightsReconciler) createOrUpdateProxyService(ctx context.Context, svc return nil } -func (r *InsightsReconciler) getProxyPodSpec() *corev1.PodSpec { +const ( + defaultProxyCPURequest = "50m" + defaultProxyCPULimit = "200m" + defaultProxyMemRequest = "64Mi" + defaultProxyMemLimit = "128Mi" +) + +func (r *InsightsReconciler) createOrUpdateProxyPodSpec(deploy *appsv1.Deployment) { privEscalation := false nonRoot := true readOnlyMode := int32(0440) - return &corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: ProxyDeploymentName, - Image: r.proxyImageTag, - Env: []corev1.EnvVar{ - { - Name: "THREESCALE_CONFIG_FILE", - Value: "/tmp/gateway-configuration-volume/config.json", - }, - }, - VolumeMounts: []corev1.VolumeMount{ - { - Name: "gateway-configuration-volume", - MountPath: "/tmp/gateway-configuration-volume", - ReadOnly: true, - }, - }, - Ports: []corev1.ContainerPort{ - { - Name: "proxy", - ContainerPort: 8080, - }, - { - Name: "management", - ContainerPort: 8090, - }, - { - Name: "metrics", - ContainerPort: 9421, - }, - }, - Resources: corev1.ResourceRequirements{}, // TODO - SecurityContext: &corev1.SecurityContext{ - AllowPrivilegeEscalation: &privEscalation, - Capabilities: &corev1.Capabilities{ - Drop: []corev1.Capability{constants.CapabilityAll}, - }, - }, - LivenessProbe: &corev1.Probe{ - InitialDelaySeconds: 10, - TimeoutSeconds: 5, - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/status/live", - Port: intstr.FromInt(8090), - }, - }, - }, - ReadinessProbe: &corev1.Probe{ - InitialDelaySeconds: 15, - PeriodSeconds: 30, - TimeoutSeconds: 5, - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/status/ready", - Port: intstr.FromInt(8090), - }, - }, - }, + podSpec := &deploy.Spec.Template.Spec + // Create the container if it doesn't exist + var container *corev1.Container + if deploy.CreationTimestamp.IsZero() { + podSpec.Containers = []corev1.Container{{}} + } + container = &podSpec.Containers[0] + + // Set fields that are hard-coded by operator + container.Name = ProxyDeploymentName + container.Image = r.proxyImageTag + container.Env = []corev1.EnvVar{ + { + Name: "THREESCALE_CONFIG_FILE", + Value: "/tmp/gateway-configuration-volume/config.json", + }, + } + container.VolumeMounts = []corev1.VolumeMount{ + { + Name: "gateway-configuration-volume", + MountPath: "/tmp/gateway-configuration-volume", + ReadOnly: true, + }, + } + container.Ports = []corev1.ContainerPort{ + { + Name: "proxy", + ContainerPort: 8080, + }, + { + Name: "management", + ContainerPort: 8090, + }, + { + Name: "metrics", + ContainerPort: 9421, + }, + } + container.SecurityContext = &corev1.SecurityContext{ + AllowPrivilegeEscalation: &privEscalation, + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{constants.CapabilityAll}, + }, + } + container.LivenessProbe = &corev1.Probe{ + InitialDelaySeconds: 10, + TimeoutSeconds: 5, + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/status/live", + Port: intstr.FromInt(8090), }, }, - Volumes: []corev1.Volume{ // TODO detect change and redeploy - { - Name: "gateway-configuration-volume", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: ProxySecretName, - Items: []corev1.KeyToPath{ - { - Key: "config.json", - Path: "config.json", - Mode: &readOnlyMode, - }, + } + container.ReadinessProbe = &corev1.Probe{ + InitialDelaySeconds: 15, + PeriodSeconds: 30, + TimeoutSeconds: 5, + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/status/ready", + Port: intstr.FromInt(8090), + }, + }, + } + + // Set resource requirements only on creation, this allows + // the user to modify them if they wish + if deploy.CreationTimestamp.IsZero() { + container.Resources = corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(defaultProxyCPURequest), + corev1.ResourceMemory: resource.MustParse(defaultProxyMemRequest), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(defaultProxyCPULimit), + corev1.ResourceMemory: resource.MustParse(defaultProxyMemLimit), + }, + } + } + + podSpec.Volumes = []corev1.Volume{ // TODO detect change and redeploy + { + Name: "gateway-configuration-volume", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: ProxySecretName, + Items: []corev1.KeyToPath{ + { + Key: "config.json", + Path: "config.json", + Mode: &readOnlyMode, }, }, }, }, }, - SecurityContext: &corev1.PodSecurityContext{ - RunAsNonRoot: &nonRoot, - SeccompProfile: common.SeccompProfile(true), - }, + } + podSpec.SecurityContext = &corev1.PodSecurityContext{ + RunAsNonRoot: &nonRoot, + SeccompProfile: common.SeccompProfile(true), } } diff --git a/internal/controllers/insights/insights_controller_test.go b/internal/controllers/insights/insights_controller_test.go index 32c4da1c9..1494ff1d4 100644 --- a/internal/controllers/insights/insights_controller_test.go +++ b/internal/controllers/insights/insights_controller_test.go @@ -61,6 +61,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -179,7 +180,8 @@ var _ = Describe("InsightsController", func() { Expect(actualContainer.LivenessProbe).To(Equal(expectedContainer.LivenessProbe)) Expect(actualContainer.StartupProbe).To(Equal(expectedContainer.StartupProbe)) Expect(actualContainer.SecurityContext).To(Equal(expectedContainer.SecurityContext)) - Expect(actualContainer.Resources).To(Equal(expectedContainer.Resources)) + + test.ExpectResourceRequirements(&actualContainer.Resources, &expectedContainer.Resources) }) It("should create the proxy service", func() { expected := t.NewInsightsProxyService() @@ -199,6 +201,81 @@ var _ = Describe("InsightsController", func() { Expect(actual.Spec.Ports).To(ConsistOf(expected.Spec.Ports)) }) }) + Context("with a proxy domain", func() { + BeforeEach(func() { + t.EnvInsightsProxyDomain = &[]string{"proxy.example.com"}[0] + }) + JustBeforeEach(func() { + result, err := t.reconcile() + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + }) + It("should create the APICast config secret", func() { + expected := t.NewInsightsProxySecretWithProxyDomain() + actual := &corev1.Secret{} + err := t.client.Get(context.Background(), types.NamespacedName{ + Name: expected.Name, + Namespace: expected.Namespace, + }, actual) + Expect(err).ToNot(HaveOccurred()) + + Expect(actual.Labels).To(Equal(expected.Labels)) + Expect(actual.Annotations).To(Equal(expected.Annotations)) + Expect(metav1.IsControlledBy(actual, t.NewProxyConfigMap())).To(BeTrue()) + Expect(actual.StringData).To(HaveLen(1)) + Expect(actual.StringData).To(HaveKey("config.json")) + Expect(actual.StringData["config.json"]).To(MatchJSON(expected.StringData["config.json"])) + }) + }) + }) + Context("updating the deployment", func() { + BeforeEach(func() { + t.objs = append(t.objs, + t.NewInsightsProxyDeployment(), + t.NewInsightsProxySecret(), + t.NewInsightsProxyService(), + ) + }) + Context("with resource requirements", func() { + var resources *corev1.ResourceRequirements + + BeforeEach(func() { + resources = &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("500m"), + corev1.ResourceMemory: resource.MustParse("256Mi"), + }, + } + }) + JustBeforeEach(func() { + // Fetch the deployment + deploy := t.getProxyDeployment() + + // Change the resource requirements + deploy.Spec.Template.Spec.Containers[0].Resources = *resources + + // Update the deployment + err := t.client.Update(context.Background(), deploy) + Expect(err).ToNot(HaveOccurred()) + + // Reconcile again + result, err := t.reconcile() + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + }) + It("should leave the custom resource requirements", func() { + // Fetch the deployment again + deploy := t.getProxyDeployment() + + // Check the resource requirements + actual := deploy.Spec.Template.Spec.Containers[0].Resources + test.ExpectResourceRequirements(&actual, resources) + }) + }) }) }) }) @@ -207,3 +284,13 @@ func (t *insightsTestInput) reconcile() (reconcile.Result, error) { req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "insights-proxy", Namespace: t.Namespace}} return t.controller.Reconcile(context.Background(), req) } + +func (t *insightsTestInput) getProxyDeployment() *appsv1.Deployment { + deploy := t.NewInsightsProxyDeployment() + err := t.client.Get(context.Background(), types.NamespacedName{ + Name: deploy.Name, + Namespace: deploy.Namespace, + }, deploy) + Expect(err).ToNot(HaveOccurred()) + return deploy +} diff --git a/internal/controllers/insights/test/resources.go b/internal/controllers/insights/test/resources.go index 500094f90..9fce9ee51 100644 --- a/internal/controllers/insights/test/resources.go +++ b/internal/controllers/insights/test/resources.go @@ -56,12 +56,14 @@ import ( "github.com/cryostatio/cryostat-operator/internal/test" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" ) type InsightsTestResources struct { *test.TestResources + Resources *corev1.ResourceRequirements } func (r *InsightsTestResources) NewGlobalPullSecret() *corev1.Secret { @@ -178,7 +180,90 @@ func (r *InsightsTestResources) NewInsightsProxySecret() *corev1.Secret { } } +func (r *InsightsTestResources) NewInsightsProxySecretWithProxyDomain() *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "apicastconf", + Namespace: r.Namespace, + }, + StringData: map[string]string{ + "config.json": fmt.Sprintf(`{ + "services": [ + { + "id": "1", + "backend_version": "1", + "proxy": { + "hosts": ["insights-proxy","insights-proxy.%s.svc.cluster.local"], + "api_backend": "https://insights.example.com:443/", + "backend": { "endpoint": "http://127.0.0.1:8081", "host": "backend" }, + "policy_chain": [ + { + "name": "default_credentials", + "version": "builtin", + "configuration": { + "auth_type": "user_key", + "user_key": "dummy_key" + } + }, + { + "name": "apicast.policy.http_proxy", + "configuration": { + "https_proxy": "http://proxy.example.com/", + "http_proxy": "http://proxy.example.com/" + } + }, + { + "name": "headers", + "version": "builtin", + "configuration": { + "request": [ + { + "op": "set", + "header": "Authorization", + "value_type": "plain", + "value": "world" + } + ] + } + }, + { + "name": "apicast.policy.apicast" + } + ], + "proxy_rules": [ + { + "http_method": "POST", + "pattern": "/", + "metric_system_name": "hits", + "delta": 1, + "parameters": [], + "querystring_parameters": {} + } + ] + } + } + ] + }`, r.Namespace), + }, + } +} + func (r *InsightsTestResources) NewInsightsProxyDeployment() *appsv1.Deployment { + var resources *corev1.ResourceRequirements + if r.Resources != nil { + resources = r.Resources + } else { + resources = &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("200m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + } + } return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: "insights-proxy", @@ -231,7 +316,7 @@ func (r *InsightsTestResources) NewInsightsProxyDeployment() *appsv1.Deployment ContainerPort: 9421, }, }, - Resources: corev1.ResourceRequirements{}, // TODO + Resources: *resources, SecurityContext: &corev1.SecurityContext{ AllowPrivilegeEscalation: &[]bool{false}[0], Capabilities: &corev1.Capabilities{ diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 956b9ccf0..b02b8d3d1 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -2908,7 +2908,7 @@ func (t *cryostatTestInput) checkCoreContainer(container *corev1.Container, ingr Expect(container.StartupProbe).To(Equal(t.NewCoreStartupProbe())) Expect(container.SecurityContext).To(Equal(securityContext)) - checkResourceRequirements(&container.Resources, resources) + test.ExpectResourceRequirements(&container.Resources, resources) } func (t *cryostatTestInput) checkGrafanaContainer(container *corev1.Container, resources *corev1.ResourceRequirements, securityContext *corev1.SecurityContext) { @@ -2925,7 +2925,7 @@ func (t *cryostatTestInput) checkGrafanaContainer(container *corev1.Container, r Expect(container.LivenessProbe).To(Equal(t.NewGrafanaLivenessProbe())) Expect(container.SecurityContext).To(Equal(securityContext)) - checkResourceRequirements(&container.Resources, resources) + test.ExpectResourceRequirements(&container.Resources, resources) } func (t *cryostatTestInput) checkDatasourceContainer(container *corev1.Container, resources *corev1.ResourceRequirements, securityContext *corev1.SecurityContext) { @@ -2942,7 +2942,7 @@ func (t *cryostatTestInput) checkDatasourceContainer(container *corev1.Container Expect(container.LivenessProbe).To(Equal(t.NewDatasourceLivenessProbe())) Expect(container.SecurityContext).To(Equal(securityContext)) - checkResourceRequirements(&container.Resources, resources) + test.ExpectResourceRequirements(&container.Resources, resources) } func (t *cryostatTestInput) checkReportsContainer(container *corev1.Container, resources *corev1.ResourceRequirements, securityContext *corev1.SecurityContext) { @@ -2958,7 +2958,7 @@ func (t *cryostatTestInput) checkReportsContainer(container *corev1.Container, r Expect(container.LivenessProbe).To(Equal(t.NewReportsLivenessProbe())) Expect(container.SecurityContext).To(Equal(securityContext)) - checkResourceRequirements(&container.Resources, resources) + test.ExpectResourceRequirements(&container.Resources, resources) } func (t *cryostatTestInput) checkCoreHasEnvironmentVariables(expectedEnvVars []corev1.EnvVar) { @@ -2972,43 +2972,6 @@ func (t *cryostatTestInput) checkCoreHasEnvironmentVariables(expectedEnvVars []c Expect(coreContainer.Env).To(ContainElements(expectedEnvVars)) } -func checkResourceRequirements(containerResource, expectedResource *corev1.ResourceRequirements) { - // Containers must have resource requests - Expect(containerResource.Requests).ToNot(BeNil()) - - requestCpu, requestCpuFound := containerResource.Requests[corev1.ResourceCPU] - expectedRequestCpu := expectedResource.Requests[corev1.ResourceCPU] - Expect(requestCpuFound).To(BeTrue()) - Expect(requestCpu.Equal(expectedRequestCpu)).To(BeTrue()) - - requestMemory, requestMemoryFound := containerResource.Requests[corev1.ResourceMemory] - expectedRequestMemory := expectedResource.Requests[corev1.ResourceMemory] - Expect(requestMemoryFound).To(BeTrue()) - Expect(requestMemory.Equal(expectedRequestMemory)).To(BeTrue()) - - if expectedResource.Limits == nil { - Expect(containerResource.Limits).To(BeNil()) - } else { - Expect(containerResource.Limits).ToNot(BeNil()) - - limitCpu, limitCpuFound := containerResource.Limits[corev1.ResourceCPU] - expectedLimitCpu, expectedLimitCpuFound := expectedResource.Limits[corev1.ResourceCPU] - - Expect(limitCpuFound).To(Equal(expectedLimitCpuFound)) - if expectedLimitCpuFound { - Expect(limitCpu.Equal(expectedLimitCpu)).To(BeTrue()) - } - - limitMemory, limitMemoryFound := containerResource.Limits[corev1.ResourceMemory] - expectedlimitMemory, expectedLimitMemoryFound := expectedResource.Limits[corev1.ResourceMemory] - - Expect(limitMemoryFound).To(Equal(expectedLimitMemoryFound)) - if expectedLimitCpuFound { - Expect(limitMemory.Equal(expectedlimitMemory)).To(BeTrue()) - } - } -} - func (t *cryostatTestInput) getCryostatInstance() *model.CryostatInstance { cr, err := t.lookupCryostatInstance() Expect(err).ToNot(HaveOccurred()) diff --git a/internal/test/expect.go b/internal/test/expect.go new file mode 100644 index 000000000..b77a19968 --- /dev/null +++ b/internal/test/expect.go @@ -0,0 +1,57 @@ +// Copyright The Cryostat Authors. +// +// 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 test + +import ( + "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" +) + +func ExpectResourceRequirements(containerResource, expectedResource *corev1.ResourceRequirements) { + // Containers must have resource requests + gomega.Expect(containerResource.Requests).ToNot(gomega.BeNil()) + + requestCpu, requestCpuFound := containerResource.Requests[corev1.ResourceCPU] + expectedRequestCpu := expectedResource.Requests[corev1.ResourceCPU] + gomega.Expect(requestCpuFound).To(gomega.BeTrue()) + gomega.Expect(requestCpu.Equal(expectedRequestCpu)).To(gomega.BeTrue()) + + requestMemory, requestMemoryFound := containerResource.Requests[corev1.ResourceMemory] + expectedRequestMemory := expectedResource.Requests[corev1.ResourceMemory] + gomega.Expect(requestMemoryFound).To(gomega.BeTrue()) + gomega.Expect(requestMemory.Equal(expectedRequestMemory)).To(gomega.BeTrue()) + + if expectedResource.Limits == nil { + gomega.Expect(containerResource.Limits).To(gomega.BeNil()) + } else { + gomega.Expect(containerResource.Limits).ToNot(gomega.BeNil()) + + limitCpu, limitCpuFound := containerResource.Limits[corev1.ResourceCPU] + expectedLimitCpu, expectedLimitCpuFound := expectedResource.Limits[corev1.ResourceCPU] + + gomega.Expect(limitCpuFound).To(gomega.Equal(expectedLimitCpuFound)) + if expectedLimitCpuFound { + gomega.Expect(limitCpu.Equal(expectedLimitCpu)).To(gomega.BeTrue()) + } + + limitMemory, limitMemoryFound := containerResource.Limits[corev1.ResourceMemory] + expectedlimitMemory, expectedLimitMemoryFound := expectedResource.Limits[corev1.ResourceMemory] + + gomega.Expect(limitMemoryFound).To(gomega.Equal(expectedLimitMemoryFound)) + if expectedLimitCpuFound { + gomega.Expect(limitMemory.Equal(expectedlimitMemory)).To(gomega.BeTrue()) + } + } +} From d2f32cff90aba14de0d07028221efda6be0e28b6 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Mon, 6 Nov 2023 14:36:39 -0500 Subject: [PATCH 14/19] Fix license --- internal/controllers/insights/apicast.go | 36 ------------------- internal/controllers/insights/insights.go | 36 ------------------- .../insights/insights_controller.go | 36 ------------------- .../insights/insights_controller_test.go | 36 ------------------- .../insights/insights_controller_unit_test.go | 36 ------------------- .../insights/insights_suite_test.go | 36 ------------------- internal/controllers/insights/setup.go | 36 ------------------- internal/controllers/insights/setup_test.go | 36 ------------------- internal/controllers/insights/test/manager.go | 36 ------------------- .../controllers/insights/test/resources.go | 36 ------------------- internal/controllers/insights/test/utils.go | 36 ------------------- 11 files changed, 396 deletions(-) diff --git a/internal/controllers/insights/apicast.go b/internal/controllers/insights/apicast.go index 625e9012c..0175706a7 100644 --- a/internal/controllers/insights/apicast.go +++ b/internal/controllers/insights/apicast.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package insights import ( diff --git a/internal/controllers/insights/insights.go b/internal/controllers/insights/insights.go index c0d9f4d85..0018ec45e 100644 --- a/internal/controllers/insights/insights.go +++ b/internal/controllers/insights/insights.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package insights import ( diff --git a/internal/controllers/insights/insights_controller.go b/internal/controllers/insights/insights_controller.go index 7ec275b0c..94950a8e8 100644 --- a/internal/controllers/insights/insights_controller.go +++ b/internal/controllers/insights/insights_controller.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package insights import ( diff --git a/internal/controllers/insights/insights_controller_test.go b/internal/controllers/insights/insights_controller_test.go index 1494ff1d4..3064b5689 100644 --- a/internal/controllers/insights/insights_controller_test.go +++ b/internal/controllers/insights/insights_controller_test.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package insights_test import ( diff --git a/internal/controllers/insights/insights_controller_unit_test.go b/internal/controllers/insights/insights_controller_unit_test.go index d27848008..ccaf682e2 100644 --- a/internal/controllers/insights/insights_controller_unit_test.go +++ b/internal/controllers/insights/insights_controller_unit_test.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package insights import ( diff --git a/internal/controllers/insights/insights_suite_test.go b/internal/controllers/insights/insights_suite_test.go index e47d78102..35a3d501b 100644 --- a/internal/controllers/insights/insights_suite_test.go +++ b/internal/controllers/insights/insights_suite_test.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package insights_test import ( diff --git a/internal/controllers/insights/setup.go b/internal/controllers/insights/setup.go index e4783efde..efb452723 100644 --- a/internal/controllers/insights/setup.go +++ b/internal/controllers/insights/setup.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package insights import ( diff --git a/internal/controllers/insights/setup_test.go b/internal/controllers/insights/setup_test.go index 7759b0e6b..216a326d7 100644 --- a/internal/controllers/insights/setup_test.go +++ b/internal/controllers/insights/setup_test.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package insights_test import ( diff --git a/internal/controllers/insights/test/manager.go b/internal/controllers/insights/test/manager.go index c34641c24..ebcd764e7 100644 --- a/internal/controllers/insights/test/manager.go +++ b/internal/controllers/insights/test/manager.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package test import ( diff --git a/internal/controllers/insights/test/resources.go b/internal/controllers/insights/test/resources.go index 9fce9ee51..4c11bf56e 100644 --- a/internal/controllers/insights/test/resources.go +++ b/internal/controllers/insights/test/resources.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package test import ( diff --git a/internal/controllers/insights/test/utils.go b/internal/controllers/insights/test/utils.go index b8df553f1..9b27c685b 100644 --- a/internal/controllers/insights/test/utils.go +++ b/internal/controllers/insights/test/utils.go @@ -12,42 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Copyright The Cryostat Authors -// -// The Universal Permissive License (UPL), Version 1.0 -// -// Subject to the condition set forth below, permission is hereby granted to any -// person obtaining a copy of this software, associated documentation and/or data -// (collectively the "Software"), free of charge and under any and all copyright -// rights in the Software, and any and all patent rights owned or freely -// licensable by each licensor hereunder covering either (i) the unmodified -// Software as contributed to or provided by such licensor, or (ii) the Larger -// Works (as defined below), to deal in both -// -// (a) the Software, and -// (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if -// one is included with the Software (each a "Larger Work" to which the Software -// is contributed by such licensors), -// -// without restriction, including without limitation the rights to copy, create -// derivative works of, display, perform, and distribute the Software and make, -// use, sell, offer for sale, import, export, have made, and have sold the -// Software and the Larger Work(s), and to sublicense the foregoing rights on -// either these or other terms. -// -// This license is subject to the following condition: -// The above copyright notice and either this complete permission notice or at -// a minimum a reference to the UPL must be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - package test import ( From f6def2b544679345a475e27bcd66f29195b7edc8 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Mon, 6 Nov 2023 14:47:07 -0500 Subject: [PATCH 15/19] Check the rest of the deployment too --- .../insights/insights_controller_test.go | 61 ++++++++++--------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/internal/controllers/insights/insights_controller_test.go b/internal/controllers/insights/insights_controller_test.go index 3064b5689..b982bdefd 100644 --- a/internal/controllers/insights/insights_controller_test.go +++ b/internal/controllers/insights/insights_controller_test.go @@ -122,30 +122,7 @@ var _ = Describe("InsightsController", func() { }, actual) Expect(err).ToNot(HaveOccurred()) - Expect(actual.Labels).To(Equal(expected.Labels)) - Expect(actual.Annotations).To(Equal(expected.Annotations)) - Expect(metav1.IsControlledBy(actual, t.NewProxyConfigMap())).To(BeTrue()) - Expect(actual.Spec.Selector).To(Equal(expected.Spec.Selector)) - - expectedTemplate := expected.Spec.Template - actualTemplate := actual.Spec.Template - Expect(actualTemplate.Labels).To(Equal(expectedTemplate.Labels)) - Expect(actualTemplate.Annotations).To(Equal(expectedTemplate.Annotations)) - Expect(actualTemplate.Spec.SecurityContext).To(Equal(expectedTemplate.Spec.SecurityContext)) - Expect(actualTemplate.Spec.Volumes).To(Equal(expectedTemplate.Spec.Volumes)) - - Expect(actualTemplate.Spec.Containers).To(HaveLen(1)) - expectedContainer := expectedTemplate.Spec.Containers[0] - actualContainer := actualTemplate.Spec.Containers[0] - Expect(actualContainer.Ports).To(ConsistOf(expectedContainer.Ports)) - Expect(actualContainer.Env).To(ConsistOf(expectedContainer.Env)) - Expect(actualContainer.EnvFrom).To(ConsistOf(expectedContainer.EnvFrom)) - Expect(actualContainer.VolumeMounts).To(ConsistOf(expectedContainer.VolumeMounts)) - Expect(actualContainer.LivenessProbe).To(Equal(expectedContainer.LivenessProbe)) - Expect(actualContainer.StartupProbe).To(Equal(expectedContainer.StartupProbe)) - Expect(actualContainer.SecurityContext).To(Equal(expectedContainer.SecurityContext)) - - test.ExpectResourceRequirements(&actualContainer.Resources, &expectedContainer.Resources) + t.checkProxyDeployment(actual, expected) }) It("should create the proxy service", func() { expected := t.NewInsightsProxyService() @@ -233,11 +210,12 @@ var _ = Describe("InsightsController", func() { }) It("should leave the custom resource requirements", func() { // Fetch the deployment again - deploy := t.getProxyDeployment() + actual := t.getProxyDeployment() - // Check the resource requirements - actual := deploy.Spec.Template.Spec.Containers[0].Resources - test.ExpectResourceRequirements(&actual, resources) + // Check only resource requirements differ from defaults + t.Resources = resources + expected := t.NewInsightsProxyDeployment() + t.checkProxyDeployment(actual, expected) }) }) }) @@ -258,3 +236,30 @@ func (t *insightsTestInput) getProxyDeployment() *appsv1.Deployment { Expect(err).ToNot(HaveOccurred()) return deploy } + +func (t *insightsTestInput) checkProxyDeployment(actual, expected *appsv1.Deployment) { + Expect(actual.Labels).To(Equal(expected.Labels)) + Expect(actual.Annotations).To(Equal(expected.Annotations)) + Expect(metav1.IsControlledBy(actual, t.NewProxyConfigMap())).To(BeTrue()) + Expect(actual.Spec.Selector).To(Equal(expected.Spec.Selector)) + + expectedTemplate := expected.Spec.Template + actualTemplate := actual.Spec.Template + Expect(actualTemplate.Labels).To(Equal(expectedTemplate.Labels)) + Expect(actualTemplate.Annotations).To(Equal(expectedTemplate.Annotations)) + Expect(actualTemplate.Spec.SecurityContext).To(Equal(expectedTemplate.Spec.SecurityContext)) + Expect(actualTemplate.Spec.Volumes).To(Equal(expectedTemplate.Spec.Volumes)) + + Expect(actualTemplate.Spec.Containers).To(HaveLen(1)) + expectedContainer := expectedTemplate.Spec.Containers[0] + actualContainer := actualTemplate.Spec.Containers[0] + Expect(actualContainer.Ports).To(ConsistOf(expectedContainer.Ports)) + Expect(actualContainer.Env).To(ConsistOf(expectedContainer.Env)) + Expect(actualContainer.EnvFrom).To(ConsistOf(expectedContainer.EnvFrom)) + Expect(actualContainer.VolumeMounts).To(ConsistOf(expectedContainer.VolumeMounts)) + Expect(actualContainer.LivenessProbe).To(Equal(expectedContainer.LivenessProbe)) + Expect(actualContainer.StartupProbe).To(Equal(expectedContainer.StartupProbe)) + Expect(actualContainer.SecurityContext).To(Equal(expectedContainer.SecurityContext)) + + test.ExpectResourceRequirements(&actualContainer.Resources, &expectedContainer.Resources) +} From a8c2178fbb6987bb39cb5cc839e98229c5e82002 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Mon, 6 Nov 2023 14:50:18 -0500 Subject: [PATCH 16/19] Update log message --- internal/controllers/insights/setup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controllers/insights/setup.go b/internal/controllers/insights/setup.go index efb452723..b5eca7cce 100644 --- a/internal/controllers/insights/setup.go +++ b/internal/controllers/insights/setup.go @@ -51,7 +51,7 @@ func (i *InsightsIntegration) Setup() (*url.URL, error) { namespace := i.getOperatorNamespace() // This will happen when running the operator locally if len(namespace) == 0 { - i.Log.Info("Operator namespace not detected, disabling Insights integration") + i.Log.Info("Operator namespace not detected") return nil, nil } From 10b03e624f39344176f6fdc858235a3413a9e8c2 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Tue, 7 Nov 2023 14:11:54 -0500 Subject: [PATCH 17/19] Add Bearer to Authentication header --- internal/controllers/insights/apicast.go | 2 +- internal/controllers/insights/test/resources.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controllers/insights/apicast.go b/internal/controllers/insights/apicast.go index 0175706a7..4fabf60bd 100644 --- a/internal/controllers/insights/apicast.go +++ b/internal/controllers/insights/apicast.go @@ -62,7 +62,7 @@ var apiCastConfigTemplate = template.Must(template.New("").Parse(`{ "op": "set", "header": "Authorization", "value_type": "plain", - "value": "{{ .HeaderValue }}" + "value": "Bearer {{ .HeaderValue }}" } ] } diff --git a/internal/controllers/insights/test/resources.go b/internal/controllers/insights/test/resources.go index 4c11bf56e..c8ada15a3 100644 --- a/internal/controllers/insights/test/resources.go +++ b/internal/controllers/insights/test/resources.go @@ -117,7 +117,7 @@ func (r *InsightsTestResources) NewInsightsProxySecret() *corev1.Secret { "op": "set", "header": "Authorization", "value_type": "plain", - "value": "world" + "value": "Bearer world" } ] } @@ -185,7 +185,7 @@ func (r *InsightsTestResources) NewInsightsProxySecretWithProxyDomain() *corev1. "op": "set", "header": "Authorization", "value_type": "plain", - "value": "world" + "value": "Bearer world" } ] } From dc58dc44858365978cd76840865e51057eb9b3ce Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Tue, 7 Nov 2023 15:18:06 -0500 Subject: [PATCH 18/19] Fix AllNamespaces install mode handling --- internal/controllers/reconciler.go | 7 +++++-- internal/main.go | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 61cc8f1f2..30b8f4b39 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -310,8 +310,11 @@ func (r *Reconciler) setupWithManager(mgr ctrl.Manager, impl reconcile.Reconcile c := ctrl.NewControllerManagedBy(mgr). For(r.objectType) - if r.isNamespaced { - // TODO remove this once only AllNamespace mode is supported + // Filter watch to specified namespaces only if the CRD is namespaced and + // we're not running in AllNamespace mode + // TODO remove this once only AllNamespace mode is supported + if r.isNamespaced && len(r.Namespaces) > 0 { + r.Log.Info(fmt.Sprintf("Adding EventFilter for namespaces: %v", r.Namespaces)) c = c.WithEventFilter(namespaceEventFilter(mgr.GetScheme(), r.Namespaces)) } diff --git a/internal/main.go b/internal/main.go index 155a39d49..efe42ab3a 100644 --- a/internal/main.go +++ b/internal/main.go @@ -85,7 +85,10 @@ func main() { setupLog.Error(err, "unable to get WatchNamespace, "+ "the manager will watch and manage resources in all namespaces") } - namespaces := strings.Split(watchNamespace, ",") + namespaces := []string{} + if len(watchNamespace) > 0 { + namespaces = append(namespaces, strings.Split(watchNamespace, ",")...) + } ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) From e11c401d5f4865c19475ac4d449aa68422d2efae Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Tue, 7 Nov 2023 15:18:26 -0500 Subject: [PATCH 19/19] Regenerate bundle --- ...yostat-operator.clusterserviceversion.yaml | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 3e368e05d..2780957ba 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -54,7 +54,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:2.5.0-dev - createdAt: "2023-10-11T14:49:05Z" + createdAt: "2023-11-07T20:18:21Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { @@ -879,6 +879,15 @@ spec: spec: clusterPermissions: - rules: + - apiGroups: + - "" + resources: + - configmaps + - configmaps/finalizers + - secrets + - services + verbs: + - '*' - apiGroups: - "" resources: @@ -916,6 +925,13 @@ spec: - statefulsets verbs: - '*' + - apiGroups: + - apps + resources: + - deployments + - deployments/finalizers + verbs: + - '*' - apiGroups: - apps.openshift.io resources: @@ -1084,6 +1100,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.annotations['olm.targetNamespaces'] + - name: NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace image: quay.io/cryostat/cryostat-operator:2.5.0-dev imagePullPolicy: Always livenessProbe: