From 8efd553b0497487ac84269cb5c27835b00a28ba2 Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Wed, 16 Dec 2020 18:54:25 -0300 Subject: [PATCH 01/10] Move admission logic to scaffolded webhook Signed-off-by: Lucas Caparelli --- .../validation => api/v1alpha1}/defaults.go | 18 +- api/v1alpha1/mutation.go | 223 +++++++++++++ api/v1alpha1/mutation_test.go | 1 + api/v1alpha1/nexus_webhook.go | 78 +++++ api/v1alpha1/validation.go | 101 ++++++ .../v1alpha1}/validation_test.go | 158 +++++---- .../resource/deployment/deployment_test.go | 25 +- .../nexus/resource/deployment/manager_test.go | 15 +- .../nexus/resource/networking/manager.go | 2 +- .../nexus/resource/persistence/pvc_test.go | 5 +- .../nexus/resource/validation/events.go | 34 -- .../nexus/resource/validation/events_test.go | 48 --- .../nexus/resource/validation/validation.go | 304 ------------------ controllers/nexus/update/log.go | 3 +- controllers/nexus/update/monitor.go | 8 +- controllers/nexus_controller.go | 56 +--- go.sum | 1 + main.go | 6 + pkg/framework/fetcher.go | 7 +- pkg/framework/fetcher_test.go | 4 +- .../nexus/update => pkg/framework}/tags.go | 10 +- .../update => pkg/framework}/tags_test.go | 6 +- pkg/test/client.go | 4 + 23 files changed, 547 insertions(+), 570 deletions(-) rename {controllers/nexus/resource/validation => api/v1alpha1}/defaults.go (88%) create mode 100644 api/v1alpha1/mutation.go create mode 100644 api/v1alpha1/mutation_test.go create mode 100644 api/v1alpha1/nexus_webhook.go create mode 100644 api/v1alpha1/validation.go rename {controllers/nexus/resource/validation => api/v1alpha1}/validation_test.go (69%) delete mode 100644 controllers/nexus/resource/validation/events.go delete mode 100644 controllers/nexus/resource/validation/events_test.go delete mode 100644 controllers/nexus/resource/validation/validation.go rename {controllers/nexus/update => pkg/framework}/tags.go (96%) rename {controllers/nexus/update => pkg/framework}/tags_test.go (97%) diff --git a/controllers/nexus/resource/validation/defaults.go b/api/v1alpha1/defaults.go similarity index 88% rename from controllers/nexus/resource/validation/defaults.go rename to api/v1alpha1/defaults.go index 219f550f..4b09db28 100644 --- a/controllers/nexus/resource/validation/defaults.go +++ b/api/v1alpha1/defaults.go @@ -12,14 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -package validation +package v1alpha1 import ( corev1 "k8s.io/api/core/v1" k8sres "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/m88i/nexus-operator/api/v1alpha1" ) const ( @@ -49,7 +47,7 @@ var ( }, } - DefaultProbe = &v1alpha1.NexusProbe{ + DefaultProbe = &NexusProbe{ InitialDelaySeconds: probeDefaultInitialDelaySeconds, TimeoutSeconds: probeDefaultTimeoutSeconds, PeriodSeconds: probeDefaultPeriodSeconds, @@ -57,30 +55,30 @@ var ( FailureThreshold: probeDefaultFailureThreshold, } - DefaultPersistence = v1alpha1.NexusPersistence{ + DefaultPersistence = NexusPersistence{ Persistent: false, VolumeSize: DefaultVolumeSize, StorageClass: "", } - DefaultNetworking = v1alpha1.NexusNetworking{ + DefaultNetworking = NexusNetworking{ Expose: false, TLS: DefaultTLS, } - DefaultTLS = v1alpha1.NexusNetworkingTLS{ + DefaultTLS = NexusNetworkingTLS{ Mandatory: false, SecretName: "", } - DefaultUpdate = v1alpha1.NexusAutomaticUpdate{ + DefaultUpdate = NexusAutomaticUpdate{ // this isn't really the default, but we need this off for most tests anyway Disabled: true, } - AllDefaultsCommunityNexus = v1alpha1.Nexus{ + AllDefaultsCommunityNexus = Nexus{ ObjectMeta: metav1.ObjectMeta{Name: "default-community-nexus", Namespace: "default"}, - Spec: v1alpha1.NexusSpec{ + Spec: NexusSpec{ Replicas: 0, Image: NexusCommunityImage, ImagePullPolicy: "", diff --git a/api/v1alpha1/mutation.go b/api/v1alpha1/mutation.go new file mode 100644 index 00000000..0b4684bf --- /dev/null +++ b/api/v1alpha1/mutation.go @@ -0,0 +1,223 @@ +package v1alpha1 + +import ( + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + + "github.com/m88i/nexus-operator/pkg/cluster/discovery" + "github.com/m88i/nexus-operator/pkg/framework" + "github.com/m88i/nexus-operator/pkg/logger" +) + +const ( + unspecifiedExposeAsFormat = "'spec.exposeAs' left unspecified, setting to: " +) + +// mutator uses its state to know what changes to apply to a Nexus CR +type mutator struct { + nexus *Nexus + log logger.Logger + routeAvailable, ingressAvailable bool +} + +// newMutator builds a *mutator with all necessary state. +// If it fails to construct this state, it assumes safe values and logs problems. +func newMutator(nexus *Nexus) *mutator { + log := logger.GetLoggerWithResource("admission_mutation", nexus) + + routeAvailable, err := discovery.IsRouteAvailable() + if err != nil { + log.Error(err, "Unable to determine if Routes are available. Will assume they're not.") + } + + ingressAvailable, err := discovery.IsIngressAvailable() + if err != nil { + log.Error(err, "Unable to determine if v1 Ingresses are available. Will assume they're not.") + } + + legacyIngressAvailable, err := discovery.IsLegacyIngressAvailable() + if err != nil { + log.Error(err, "Unable to determine if v1beta1 Ingresses are available. Will assume they're not.") + } + + return &mutator{ + nexus: nexus, + log: log, + routeAvailable: routeAvailable, + // TODO: create a IsAnyIngressAvailable function in discovery, we don't care about which one in admission + ingressAvailable: ingressAvailable || legacyIngressAvailable, + } +} + +// mutate make all necessary changes to the Nexus resource prior to validation +func (m *mutator) mutate() { + m.mutateDeployment() + m.mutateAutomaticUpdate() + m.mutateNetworking() + m.mutatePersistence() + m.mutateSecurity() +} + +func (m *mutator) mutateDeployment() { + m.mutateReplicas() + m.mutateResources() + m.mutateImage() + m.mutateProbe() +} + +func (m *mutator) mutateReplicas() { + if m.nexus.Spec.Replicas > maxReplicas { + m.log.Warn("Number of replicas not supported", "MaxSupportedReplicas", maxReplicas, "DesiredReplicas", m.nexus.Spec.Replicas) + m.nexus.Spec.Replicas = maxReplicas + } +} + +func (m *mutator) mutateResources() { + if m.nexus.Spec.Resources.Requests == nil && m.nexus.Spec.Resources.Limits == nil { + m.nexus.Spec.Resources = DefaultResources + } +} + +func (m *mutator) mutateImage() { + if m.nexus.Spec.UseRedHatImage { + if len(m.nexus.Spec.Image) > 0 { + m.log.Warn("Nexus CR configured to the use Red Hat Certified Image, ignoring 'spec.image' field.") + } + m.nexus.Spec.Image = NexusCertifiedImage + } + if len(m.nexus.Spec.Image) == 0 { + m.nexus.Spec.Image = NexusCommunityImage + } + + if len(m.nexus.Spec.ImagePullPolicy) > 0 && + m.nexus.Spec.ImagePullPolicy != corev1.PullAlways && + m.nexus.Spec.ImagePullPolicy != corev1.PullIfNotPresent && + m.nexus.Spec.ImagePullPolicy != corev1.PullNever { + + m.log.Warn("Invalid 'spec.imagePullPolicy', unsetting the value. The pull policy will be determined by the image tag. Valid value are", "#1", corev1.PullAlways, "#2", corev1.PullIfNotPresent, "#3", corev1.PullNever) + m.nexus.Spec.ImagePullPolicy = "" + } +} + +func (m *mutator) mutateProbe() { + if m.nexus.Spec.LivenessProbe != nil { + m.nexus.Spec.LivenessProbe.FailureThreshold = + ensureMinimum(m.nexus.Spec.LivenessProbe.FailureThreshold, 1) + m.nexus.Spec.LivenessProbe.InitialDelaySeconds = + ensureMinimum(m.nexus.Spec.LivenessProbe.InitialDelaySeconds, 0) + m.nexus.Spec.LivenessProbe.PeriodSeconds = + ensureMinimum(m.nexus.Spec.LivenessProbe.PeriodSeconds, 1) + m.nexus.Spec.LivenessProbe.TimeoutSeconds = + ensureMinimum(m.nexus.Spec.LivenessProbe.TimeoutSeconds, 1) + } else { + m.nexus.Spec.LivenessProbe = DefaultProbe.DeepCopy() + } + + // SuccessThreshold for Liveness Probes must be 1 + m.nexus.Spec.LivenessProbe.SuccessThreshold = 1 + + if m.nexus.Spec.ReadinessProbe != nil { + m.nexus.Spec.ReadinessProbe.FailureThreshold = + ensureMinimum(m.nexus.Spec.ReadinessProbe.FailureThreshold, 1) + m.nexus.Spec.ReadinessProbe.InitialDelaySeconds = + ensureMinimum(m.nexus.Spec.ReadinessProbe.InitialDelaySeconds, 0) + m.nexus.Spec.ReadinessProbe.PeriodSeconds = + ensureMinimum(m.nexus.Spec.ReadinessProbe.PeriodSeconds, 1) + m.nexus.Spec.ReadinessProbe.SuccessThreshold = + ensureMinimum(m.nexus.Spec.ReadinessProbe.SuccessThreshold, 1) + m.nexus.Spec.ReadinessProbe.TimeoutSeconds = + ensureMinimum(m.nexus.Spec.ReadinessProbe.TimeoutSeconds, 1) + } else { + m.nexus.Spec.ReadinessProbe = DefaultProbe.DeepCopy() + } +} + +func (m *mutator) mutateAutomaticUpdate() { + if m.nexus.Spec.AutomaticUpdate.Disabled { + return + } + + image := strings.Split(m.nexus.Spec.Image, ":")[0] + if image != NexusCommunityImage { + m.log.Warn("Automatic Updates are enabled, but 'spec.image' is not using the community image. Disabling automatic updates", "Community Image", NexusCommunityImage) + m.nexus.Spec.AutomaticUpdate.Disabled = true + return + } + + if m.nexus.Spec.AutomaticUpdate.MinorVersion == nil { + m.log.Debug("Automatic Updates are enabled, but no minor was informed. Fetching the most recent...") + minor, err := framework.GetLatestMinor() + if err != nil { + m.log.Error(err, "Unable to fetch the most recent minor. Disabling automatic updates.") + m.nexus.Spec.AutomaticUpdate.Disabled = true + return + } + m.nexus.Spec.AutomaticUpdate.MinorVersion = &minor + } + + m.log.Debug("Fetching the latest micro from minor", "MinorVersion", *m.nexus.Spec.AutomaticUpdate.MinorVersion) + tag, ok := framework.GetLatestMicro(*m.nexus.Spec.AutomaticUpdate.MinorVersion) + if !ok { + // the informed minor doesn't exist, let's try the latest minor + m.log.Warn("Latest tag for minor version not found. Trying the latest minor instead", "Informed tag", *m.nexus.Spec.AutomaticUpdate.MinorVersion) + minor, err := framework.GetLatestMinor() + if err != nil { + m.log.Error(err, "Unable to fetch the most recent minor: %m. Disabling automatic updates.") + m.nexus.Spec.AutomaticUpdate.Disabled = true + return + } + m.log.Info("Setting 'spec.automaticUpdate.minorVersion to", "MinorTag", minor) + m.nexus.Spec.AutomaticUpdate.MinorVersion = &minor + // no need to check for the tag existence here, + // we would have gotten an error from GetLatestMinor() if it didn't + tag, _ = framework.GetLatestMicro(minor) + } + newImage := fmt.Sprintf("%s:%s", image, tag) + if newImage != m.nexus.Spec.Image { + m.log.Debug("Replacing 'spec.image'", "OldImage", m.nexus.Spec.Image, "NewImage", newImage) + m.nexus.Spec.Image = newImage + } +} + +func (m *mutator) mutateNetworking() { + if !m.nexus.Spec.Networking.Expose { + return + } + + if len(m.nexus.Spec.Networking.ExposeAs) == 0 { + if m.routeAvailable { + m.log.Info(unspecifiedExposeAsFormat, "ExposeType", RouteExposeType) + m.nexus.Spec.Networking.ExposeAs = RouteExposeType + } else if m.ingressAvailable { + m.log.Info(unspecifiedExposeAsFormat, "ExposeType", IngressExposeType) + m.nexus.Spec.Networking.ExposeAs = IngressExposeType + } else { + // we're on kubernetes < 1.14 + // try setting nodePort, validation will catch it if impossible + m.log.Info("On Kubernetes, but Ingresses are not available") + m.log.Info(unspecifiedExposeAsFormat, "ExposeType", NodePortExposeType) + m.nexus.Spec.Networking.ExposeAs = NodePortExposeType + } + } +} + +func (m *mutator) mutatePersistence() { + if m.nexus.Spec.Persistence.Persistent && len(m.nexus.Spec.Persistence.VolumeSize) == 0 { + m.nexus.Spec.Persistence.VolumeSize = DefaultVolumeSize + } +} + +func (m *mutator) mutateSecurity() { + if len(m.nexus.Spec.ServiceAccountName) == 0 { + m.nexus.Spec.ServiceAccountName = m.nexus.Name + } +} + +func ensureMinimum(value, minimum int32) int32 { + if value < minimum { + return minimum + } + return value +} diff --git a/api/v1alpha1/mutation_test.go b/api/v1alpha1/mutation_test.go new file mode 100644 index 00000000..1c267880 --- /dev/null +++ b/api/v1alpha1/mutation_test.go @@ -0,0 +1 @@ +package v1alpha1 diff --git a/api/v1alpha1/nexus_webhook.go b/api/v1alpha1/nexus_webhook.go new file mode 100644 index 00000000..5136eb97 --- /dev/null +++ b/api/v1alpha1/nexus_webhook.go @@ -0,0 +1,78 @@ +// Copyright 2020 Nexus Operator and/or its 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 v1alpha1 + +import ( + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook" + + "github.com/m88i/nexus-operator/pkg/logger" +) + +const admissionLogName = "admission" + +// log is for logging in this package. +var log = logger.GetLogger(admissionLogName) + +func (r *Nexus) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(r). + Complete() +} + +// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! + +// +kubebuilder:webhook:path=/mutate-apps-m88i-io-m88i-io-v1alpha1-nexus,mutating=true,failurePolicy=fail,groups=apps.m88i.io.m88i.io,resources=nexus,verbs=create;update,versions=v1alpha1,name=mnexus.kb.io + +var _ webhook.Defaulter = &Nexus{} + +// Default implements webhook.Defaulter so a webhook will be registered for the type +func (r *Nexus) Default() { + log = logger.GetLoggerWithResource(admissionLogName, r) + defer func() { log = logger.GetLogger(admissionLogName) }() + + log.Info("Setting defaults and making necessary changes to the Nexus") + newMutator(r).mutate() +} + +// if we ever need validation upon delete requests change verbs to "verbs=create;update;delete". +// +kubebuilder:webhook:verbs=create;update,path=/validate-apps-m88i-io-m88i-io-v1alpha1-nexus,mutating=false,failurePolicy=fail,groups=apps.m88i.io.m88i.io,resources=nexus,versions=v1alpha1,name=vnexus.kb.io + +var _ webhook.Validator = &Nexus{} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +func (r *Nexus) ValidateCreate() error { + log = logger.GetLoggerWithResource(admissionLogName, r) + defer func() { log = logger.GetLogger(admissionLogName) }() + + log.Info("Validating a new Nexus") + return newValidator(r).validate() +} + +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type +func (r *Nexus) ValidateUpdate(old runtime.Object) error { + log = logger.GetLoggerWithResource(admissionLogName, r) + defer func() { log = logger.GetLogger(admissionLogName) }() + + log.Info("Validating an update to a existing Nexus") + return newValidator(r).validate() +} + +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type +func (r *Nexus) ValidateDelete() error { + // we don't care about validation on delete requests + return nil +} diff --git a/api/v1alpha1/validation.go b/api/v1alpha1/validation.go new file mode 100644 index 00000000..9f82ba9d --- /dev/null +++ b/api/v1alpha1/validation.go @@ -0,0 +1,101 @@ +// Copyright 2020 Nexus Operator and/or its 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 v1alpha1 + +import ( + "fmt" + + "github.com/m88i/nexus-operator/pkg/cluster/discovery" + "github.com/m88i/nexus-operator/pkg/logger" +) + +// validator uses its state to check if a given Nexus CR is valid +type validator struct { + nexus *Nexus + log logger.Logger + routeAvailable, ingressAvailable bool +} + +// newValidator builds a *validator with all necessary state. If it fails to construct this state, it returns an error. +// If it fails to construct this state, it assumes safe values and logs problems. +func newValidator(nexus *Nexus) *validator { + log := logger.GetLoggerWithResource("admission_validation", nexus) + + routeAvailable, err := discovery.IsRouteAvailable() + if err != nil { + log.Error(err, "Unable to determine if Routes are available. Will assume they're not.") + } + + ingressAvailable, err := discovery.IsIngressAvailable() + if err != nil { + log.Error(err, "Unable to determine if v1 Ingresses are available. Will assume they're not.") + } + + legacyIngressAvailable, err := discovery.IsLegacyIngressAvailable() + if err != nil { + log.Error(err, "Unable to determine if v1beta1 Ingresses are available. Will assume they're not.") + } + + return &validator{ + nexus: nexus, + log: log, + routeAvailable: routeAvailable, + ingressAvailable: ingressAvailable || legacyIngressAvailable, + } +} + +// validate returns an error if the Nexus CR is invalid +func (v *validator) validate() error { + return v.validateNetworking() +} + +func (v *validator) validateNetworking() error { + if !v.nexus.Spec.Networking.Expose { + v.log.Debug("'spec.networking.expose' set to 'false', ignoring networking configuration") + return nil + } + + if !v.ingressAvailable && v.nexus.Spec.Networking.ExposeAs == IngressExposeType { + v.log.Warn("Ingresses are not available on your cluster. Make sure to be running Kubernetes > 1.14 or if you're running Openshift set ", "spec.networking.exposeAs", IngressExposeType, "Also try", NodePortExposeType) + return fmt.Errorf("ingress expose required, but unavailable") + } + + if !v.routeAvailable && v.nexus.Spec.Networking.ExposeAs == RouteExposeType { + v.log.Warn("Routes are not available on your cluster. If you're running Kubernetes 1.14 or higher try setting ", "'spec.networking.exposeAs'", IngressExposeType, "Also try", NodePortExposeType) + return fmt.Errorf("route expose required, but unavailable") + } + + if v.nexus.Spec.Networking.ExposeAs == NodePortExposeType && v.nexus.Spec.Networking.NodePort == 0 { + v.log.Warn("NodePort networking requires a port. Check the Nexus resource 'spec.networking.nodePort' parameter") + return fmt.Errorf("nodeport expose required, but no port informed") + } + + if v.nexus.Spec.Networking.ExposeAs == IngressExposeType && len(v.nexus.Spec.Networking.Host) == 0 { + v.log.Warn("Ingress networking requires a host. Check the Nexus resource 'spec.networking.host' parameter") + return fmt.Errorf("ingress expose required, but no host informed") + } + + if len(v.nexus.Spec.Networking.TLS.SecretName) > 0 && v.nexus.Spec.Networking.ExposeAs != IngressExposeType { + v.log.Warn("'spec.networking.tls.secretName' is only available when using an Ingress. Try setting ", "spec.networking.exposeAs'", IngressExposeType) + return fmt.Errorf("tls secret name informed, but using route") + } + + if v.nexus.Spec.Networking.TLS.Mandatory && v.nexus.Spec.Networking.ExposeAs != RouteExposeType { + v.log.Warn("'spec.networking.tls.mandatory' is only available when using a Route. Try setting ", "spec.networking.exposeAs'", RouteExposeType) + return fmt.Errorf("tls set to mandatory, but using ingress") + } + + return nil +} diff --git a/controllers/nexus/resource/validation/validation_test.go b/api/v1alpha1/validation_test.go similarity index 69% rename from controllers/nexus/resource/validation/validation_test.go rename to api/v1alpha1/validation_test.go index 0105e570..e49b00ca 100644 --- a/controllers/nexus/resource/validation/validation_test.go +++ b/api/v1alpha1/validation_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package validation +package v1alpha1 import ( "fmt" @@ -22,79 +22,74 @@ import ( "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" - "github.com/m88i/nexus-operator/api/v1alpha1" - "github.com/m88i/nexus-operator/controllers/nexus/update" "github.com/m88i/nexus-operator/pkg/cluster/discovery" + "github.com/m88i/nexus-operator/pkg/framework" "github.com/m88i/nexus-operator/pkg/logger" "github.com/m88i/nexus-operator/pkg/test" ) +// TODO: fix the tests +// TODO: move mutation tests to mutation_test.go + func TestNewValidator(t *testing.T) { tests := []struct { name string client *test.FakeClient - want *Validator + want *validator }{ { "On OCP", test.NewFakeClientBuilder().OnOpenshift().Build(), - &Validator{ + &validator{ routeAvailable: true, ingressAvailable: false, - ocp: true, }, }, { "On K8s, no ingress", test.NewFakeClientBuilder().Build(), - &Validator{ + &validator{ routeAvailable: false, ingressAvailable: false, - ocp: false, }, }, { "On K8s with v1beta1 ingress", test.NewFakeClientBuilder().WithLegacyIngress().Build(), - &Validator{ + &validator{ routeAvailable: false, ingressAvailable: true, - ocp: false, }, }, { "On K8s with v1 ingress", test.NewFakeClientBuilder().WithIngress().Build(), - &Validator{ + &validator{ routeAvailable: false, ingressAvailable: true, - ocp: false, }, }, } for _, tt := range tests { discovery.SetClient(tt.client) - got, err := NewValidator(tt.client, tt.client.Scheme()) - assert.Nil(t, err) - tt.want.client = tt.client - tt.want.scheme = tt.client.Scheme() - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("%s\nWant: %+v\nGot: %+v", tt.name, tt.want, got) - } + // the nexus CR is not important here + assert.Equal(t, tt.want, newValidator(nil)) } // now let's test out error - client := test.NewFakeClientBuilder().Build() + client := test.NewFakeClientBuilder().WithIngress().Build() errString := "test error" - client.SetMockErrorForOneRequest(fmt.Errorf(errString)) + client.SetMockError(fmt.Errorf(errString)) discovery.SetClient(client) - _, err := NewValidator(client, client.Scheme()) - assert.Contains(t, err.Error(), errString) + got := newValidator(nil) + // all calls to discovery failed, so all should be false + assert.False(t, got.routeAvailable) + assert.False(t, got.ingressAvailable) } func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { - minimumDefaultProbe := &v1alpha1.NexusProbe{ + minimumDefaultProbe := &NexusProbe{ InitialDelaySeconds: 0, TimeoutSeconds: 1, PeriodSeconds: 1, @@ -104,12 +99,12 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { tests := []struct { name string - input *v1alpha1.Nexus - want *v1alpha1.Nexus + input *Nexus + want *Nexus }{ { "'spec.resources' left blank", - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() nexus.Spec.Resources = corev1.ResourceRequirements{} return nexus @@ -118,13 +113,13 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { }, { "'spec.useRedHatImage' set to true and 'spec.image' not left blank", - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() nexus.Spec.UseRedHatImage = true nexus.Spec.Image = "some-image" return nexus }(), - func() *v1alpha1.Nexus { + func() *Nexus { n := AllDefaultsCommunityNexus.DeepCopy() n.Spec.UseRedHatImage = true n.Spec.Image = NexusCertifiedImage @@ -133,7 +128,7 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { }, { "'spec.useRedHatImage' set to false and 'spec.image' left blank", - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() nexus.Spec.Image = "" return nexus @@ -142,7 +137,7 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { }, { "'spec.livenessProbe.successThreshold' not equal to 1", - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() nexus.Spec.LivenessProbe.SuccessThreshold = 2 return nexus @@ -151,9 +146,9 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { }, { "'spec.livenessProbe.*' and 'spec.readinessProbe.*' don't meet minimum values", - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.LivenessProbe = &v1alpha1.NexusProbe{ + nexus.Spec.LivenessProbe = &NexusProbe{ InitialDelaySeconds: -1, TimeoutSeconds: -1, PeriodSeconds: -1, @@ -163,7 +158,7 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { nexus.Spec.ReadinessProbe = nexus.Spec.LivenessProbe.DeepCopy() return nexus }(), - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() nexus.Spec.LivenessProbe = minimumDefaultProbe.DeepCopy() nexus.Spec.ReadinessProbe = minimumDefaultProbe.DeepCopy() @@ -172,7 +167,7 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { }, { "Unset 'spec.livenessProbe' and 'spec.readinessProbe'", - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() nexus.Spec.LivenessProbe = nil nexus.Spec.ReadinessProbe = nil @@ -182,7 +177,7 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { }, { "Invalid 'spec.imagePullPolicy'", - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() nexus.Spec.ImagePullPolicy = "invalid" return nexus @@ -191,12 +186,12 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { }, { "Invalid 'spec.replicas'", - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() nexus.Spec.Replicas = 3 return nexus }(), - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() nexus.Spec.Replicas = 1 return nexus @@ -205,7 +200,7 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { } for _, tt := range tests { - v := &Validator{} + v := &validator{} got, err := v.SetDefaultsAndValidate(tt.input) assert.Nil(t, err) if !reflect.DeepEqual(got, tt.want) { @@ -216,13 +211,13 @@ func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { func TestValidator_setUpdateDefaults(t *testing.T) { client := test.NewFakeClientBuilder().Build() - nexus := &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{AutomaticUpdate: v1alpha1.NexusAutomaticUpdate{}}} + nexus := &Nexus{Spec: NexusSpec{AutomaticUpdate: NexusAutomaticUpdate{}}} nexus.Spec.Image = NexusCommunityImage - v, _ := NewValidator(client, client.Scheme()) + v, _ := newValidator(client, client.Scheme()) v.log = logger.GetLoggerWithResource("test", nexus) v.setUpdateDefaults(nexus) - latestMinor, err := update.GetLatestMinor() + latestMinor, err := framework.GetLatestMinor() if err != nil { // If we couldn't fetch the tags updates should be disabled assert.True(t, nexus.Spec.AutomaticUpdate.Disabled) @@ -231,22 +226,21 @@ func TestValidator_setUpdateDefaults(t *testing.T) { } // Now an invalid image - nexus = &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{AutomaticUpdate: v1alpha1.NexusAutomaticUpdate{}}} + nexus = &Nexus{Spec: NexusSpec{AutomaticUpdate: NexusAutomaticUpdate{}}} nexus.Spec.Image = "some-image" v.setUpdateDefaults(nexus) assert.True(t, nexus.Spec.AutomaticUpdate.Disabled) // Informed a minor which does not exist - nexus = &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{AutomaticUpdate: v1alpha1.NexusAutomaticUpdate{}}} + nexus = &Nexus{Spec: NexusSpec{AutomaticUpdate: NexusAutomaticUpdate{}}} nexus.Spec.Image = NexusCommunityImage bogusMinor := -1 nexus.Spec.AutomaticUpdate.MinorVersion = &bogusMinor v.setUpdateDefaults(nexus) - latestMinor, err = update.GetLatestMinor() + latestMinor, err = framework.GetLatestMinor() if err != nil { // If we couldn't fetch the tags updates should be disabled assert.True(t, nexus.Spec.AutomaticUpdate.Disabled) - assert.True(t, test.EventExists(client, changedNexusReason)) } else { assert.Equal(t, latestMinor, *nexus.Spec.AutomaticUpdate.MinorVersion) } @@ -258,23 +252,23 @@ func TestValidator_setNetworkingDefaults(t *testing.T) { ocp bool routeAvailable bool ingressAvailable bool - input *v1alpha1.Nexus - want *v1alpha1.Nexus + input *Nexus + want *Nexus }{ { "'spec.networking.exposeAs' left blank on OCP", true, true, false, // unimportant - func() *v1alpha1.Nexus { + func() *Nexus { n := AllDefaultsCommunityNexus.DeepCopy() n.Spec.Networking.Expose = true return n }(), - func() *v1alpha1.Nexus { + func() *Nexus { n := AllDefaultsCommunityNexus.DeepCopy() n.Spec.Networking.Expose = true - n.Spec.Networking.ExposeAs = v1alpha1.RouteExposeType + n.Spec.Networking.ExposeAs = RouteExposeType return n }(), }, @@ -283,15 +277,15 @@ func TestValidator_setNetworkingDefaults(t *testing.T) { false, false, true, - func() *v1alpha1.Nexus { + func() *Nexus { n := AllDefaultsCommunityNexus.DeepCopy() n.Spec.Networking.Expose = true return n }(), - func() *v1alpha1.Nexus { + func() *Nexus { n := AllDefaultsCommunityNexus.DeepCopy() n.Spec.Networking.Expose = true - n.Spec.Networking.ExposeAs = v1alpha1.IngressExposeType + n.Spec.Networking.ExposeAs = IngressExposeType return n }(), }, @@ -300,22 +294,22 @@ func TestValidator_setNetworkingDefaults(t *testing.T) { false, false, false, - func() *v1alpha1.Nexus { + func() *Nexus { n := AllDefaultsCommunityNexus.DeepCopy() n.Spec.Networking.Expose = true return n }(), - func() *v1alpha1.Nexus { + func() *Nexus { n := AllDefaultsCommunityNexus.DeepCopy() n.Spec.Networking.Expose = true - n.Spec.Networking.ExposeAs = v1alpha1.NodePortExposeType + n.Spec.Networking.ExposeAs = NodePortExposeType return n }(), }, } for _, tt := range tests { - v := &Validator{ + v := &validator{ routeAvailable: tt.routeAvailable, ingressAvailable: tt.ingressAvailable, ocp: tt.ocp, @@ -335,7 +329,7 @@ func TestValidator_validateNetworking(t *testing.T) { ocp bool routeAvailable bool ingressAvailable bool - input *v1alpha1.Nexus + input *Nexus wantError bool }{ { @@ -343,7 +337,7 @@ func TestValidator_validateNetworking(t *testing.T) { false, // unimportant false, // unimportant false, // unimportant - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: false}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: false}}}, false, }, { @@ -351,7 +345,7 @@ func TestValidator_validateNetworking(t *testing.T) { false, false, true, - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.IngressExposeType, Host: "example.com"}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType, Host: "example.com"}}}, false, }, { @@ -359,7 +353,7 @@ func TestValidator_validateNetworking(t *testing.T) { false, false, true, - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.IngressExposeType, Host: "example.com", TLS: v1alpha1.NexusNetworkingTLS{SecretName: "tt-secret"}}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType, Host: "example.com", TLS: NexusNetworkingTLS{SecretName: "tt-secret"}}}}, false, }, { @@ -367,7 +361,7 @@ func TestValidator_validateNetworking(t *testing.T) { false, false, false, - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.IngressExposeType, Host: "example.com"}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType, Host: "example.com"}}}, true, }, { @@ -375,7 +369,7 @@ func TestValidator_validateNetworking(t *testing.T) { false, false, true, - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.IngressExposeType}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType}}}, true, }, { @@ -383,7 +377,7 @@ func TestValidator_validateNetworking(t *testing.T) { false, false, true, - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.IngressExposeType, Host: "example.com", TLS: v1alpha1.NexusNetworkingTLS{Mandatory: true}}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType, Host: "example.com", TLS: NexusNetworkingTLS{Mandatory: true}}}}, true, }, { @@ -391,7 +385,7 @@ func TestValidator_validateNetworking(t *testing.T) { false, false, true, - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.RouteExposeType}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: RouteExposeType}}}, true, }, { @@ -399,7 +393,7 @@ func TestValidator_validateNetworking(t *testing.T) { true, true, false, - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.RouteExposeType}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: RouteExposeType}}}, false, }, { @@ -407,7 +401,7 @@ func TestValidator_validateNetworking(t *testing.T) { true, true, false, - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.RouteExposeType, TLS: v1alpha1.NexusNetworkingTLS{Mandatory: true}}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: RouteExposeType, TLS: NexusNetworkingTLS{Mandatory: true}}}}, false, }, { @@ -415,7 +409,7 @@ func TestValidator_validateNetworking(t *testing.T) { true, true, false, - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.RouteExposeType, TLS: v1alpha1.NexusNetworkingTLS{SecretName: "test-secret"}}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: RouteExposeType, TLS: NexusNetworkingTLS{SecretName: "test-secret"}}}}, true, }, { @@ -423,7 +417,7 @@ func TestValidator_validateNetworking(t *testing.T) { true, true, false, - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.IngressExposeType, Host: "example.com"}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType, Host: "example.com"}}}, true, }, { @@ -431,7 +425,7 @@ func TestValidator_validateNetworking(t *testing.T) { false, // unimportant false, // unimportant false, // unimportant - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.NodePortExposeType, NodePort: 8080}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: NodePortExposeType, NodePort: 8080}}}, false, }, { @@ -439,13 +433,13 @@ func TestValidator_validateNetworking(t *testing.T) { false, // unimportant false, // unimportant false, // unimportant - &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.NodePortExposeType}}}, + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: NodePortExposeType}}}, true, }, } for _, tt := range tests { - v := &Validator{ + v := &validator{ routeAvailable: tt.routeAvailable, ingressAvailable: tt.ingressAvailable, ocp: tt.ocp, @@ -460,17 +454,17 @@ func TestValidator_validateNetworking(t *testing.T) { func TestValidator_SetDefaultsAndValidate_Persistence(t *testing.T) { tests := []struct { name string - input *v1alpha1.Nexus - want *v1alpha1.Nexus + input *Nexus + want *Nexus }{ { "'spec.persistence.volumeSize' left blank", - func() *v1alpha1.Nexus { + func() *Nexus { n := AllDefaultsCommunityNexus.DeepCopy() n.Spec.Persistence.Persistent = true return n }(), - func() *v1alpha1.Nexus { + func() *Nexus { n := AllDefaultsCommunityNexus.DeepCopy() n.Spec.Persistence.Persistent = true n.Spec.Persistence.VolumeSize = DefaultVolumeSize @@ -479,7 +473,7 @@ func TestValidator_SetDefaultsAndValidate_Persistence(t *testing.T) { }, } for _, tt := range tests { - v := &Validator{} + v := &validator{} got, err := v.SetDefaultsAndValidate(tt.input) assert.Nil(t, err) if !reflect.DeepEqual(got, tt.want) { @@ -492,12 +486,12 @@ func TestValidator_SetDefaultsAndValidate_Security(t *testing.T) { tests := []struct { name string - input *v1alpha1.Nexus - want *v1alpha1.Nexus + input *Nexus + want *Nexus }{ { "'spec.serviceAccountName' left blank", - func() *v1alpha1.Nexus { + func() *Nexus { nexus := AllDefaultsCommunityNexus.DeepCopy() nexus.Spec.ServiceAccountName = "" return nexus @@ -506,7 +500,7 @@ func TestValidator_SetDefaultsAndValidate_Security(t *testing.T) { }, } for _, tt := range tests { - v := &Validator{} + v := &validator{} got, err := v.SetDefaultsAndValidate(tt.input) assert.Nil(t, err) if !reflect.DeepEqual(got, tt.want) { diff --git a/controllers/nexus/resource/deployment/deployment_test.go b/controllers/nexus/resource/deployment/deployment_test.go index c8aaff65..ae874db4 100644 --- a/controllers/nexus/resource/deployment/deployment_test.go +++ b/controllers/nexus/resource/deployment/deployment_test.go @@ -25,7 +25,6 @@ import ( "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/controllers/nexus/resource/meta" - "github.com/m88i/nexus-operator/controllers/nexus/resource/validation" ) func Test_newDeployment_WithoutPersistence(t *testing.T) { @@ -43,15 +42,15 @@ func Test_newDeployment_WithoutPersistence(t *testing.T) { }, // a valid Liveness Probe should have successThreshold == 1 // but we don't care about that here (this is tested on the manager's tests) - LivenessProbe: validation.DefaultProbe, - ReadinessProbe: validation.DefaultProbe, - Image: validation.NexusCommunityImage, + LivenessProbe: v1alpha1.DefaultProbe, + ReadinessProbe: v1alpha1.DefaultProbe, + Image: v1alpha1.NexusCommunityImage, }, } deployment := newDeployment(nexus) assert.Len(t, deployment.Spec.Template.Spec.Containers, 1) - assert.Equal(t, validation.NexusCommunityImage, deployment.Spec.Template.Spec.Containers[0].Image) + assert.Equal(t, v1alpha1.NexusCommunityImage, deployment.Spec.Template.Spec.Containers[0].Image) assert.Equal(t, int32(1), *deployment.Spec.Replicas) assert.Equal(t, int32(nexusContainerPort), deployment.Spec.Template.Spec.Containers[0].LivenessProbe.HTTPGet.Port.IntVal) @@ -80,8 +79,8 @@ func Test_newDeployment_WithPersistence(t *testing.T) { }, // a valid Liveness Probe should have successThreshold == 1 // but we don't care about that here (this is tested on the manager's tests) - LivenessProbe: validation.DefaultProbe, - ReadinessProbe: validation.DefaultProbe, + LivenessProbe: v1alpha1.DefaultProbe, + ReadinessProbe: v1alpha1.DefaultProbe, }, } deployment := newDeployment(nexus) @@ -161,7 +160,7 @@ func Test_customProbes(t *testing.T) { SuccessThreshold: 3, TimeoutSeconds: 15, }, - ReadinessProbe: validation.DefaultProbe, + ReadinessProbe: v1alpha1.DefaultProbe, }, } deployment := newDeployment(nexus) @@ -169,7 +168,7 @@ func Test_customProbes(t *testing.T) { assert.Len(t, deployment.Spec.Template.Spec.Containers, 1) assert.Equal(t, int32(1), deployment.Spec.Template.Spec.Containers[0].LivenessProbe.FailureThreshold) assert.Equal(t, int32(0), deployment.Spec.Template.Spec.Containers[0].LivenessProbe.InitialDelaySeconds) - assert.Equal(t, validation.DefaultProbe.InitialDelaySeconds, deployment.Spec.Template.Spec.Containers[0].ReadinessProbe.InitialDelaySeconds) + assert.Equal(t, v1alpha1.DefaultProbe.InitialDelaySeconds, deployment.Spec.Template.Spec.Containers[0].ReadinessProbe.InitialDelaySeconds) } func Test_applyJVMArgs_withRandomPassword(t *testing.T) { @@ -188,8 +187,8 @@ func Test_applyJVMArgs_withRandomPassword(t *testing.T) { GenerateRandomAdminPassword: true, // a valid Liveness Probe should have successThreshold == 1 // but we don't care about that here (this is tested on the manager's tests) - LivenessProbe: validation.DefaultProbe, - ReadinessProbe: validation.DefaultProbe, + LivenessProbe: v1alpha1.DefaultProbe, + ReadinessProbe: v1alpha1.DefaultProbe, }, } deployment := newDeployment(nexus) @@ -214,8 +213,8 @@ func Test_applyJVMArgs_withDefaultValues(t *testing.T) { AutomaticUpdate: v1alpha1.NexusAutomaticUpdate{Disabled: true}, // a valid Liveness Probe should have successThreshold == 1 // but we don't care about that here (this is tested on the manager's tests) - LivenessProbe: validation.DefaultProbe, - ReadinessProbe: validation.DefaultProbe, + LivenessProbe: v1alpha1.DefaultProbe, + ReadinessProbe: v1alpha1.DefaultProbe, }, } deployment := newDeployment(nexus) diff --git a/controllers/nexus/resource/deployment/manager_test.go b/controllers/nexus/resource/deployment/manager_test.go index eaff6a14..d3770ce7 100644 --- a/controllers/nexus/resource/deployment/manager_test.go +++ b/controllers/nexus/resource/deployment/manager_test.go @@ -27,7 +27,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/m88i/nexus-operator/api/v1alpha1" - "github.com/m88i/nexus-operator/controllers/nexus/resource/validation" "github.com/m88i/nexus-operator/pkg/logger" "github.com/m88i/nexus-operator/pkg/test" ) @@ -38,10 +37,10 @@ var ( Spec: v1alpha1.NexusSpec{ AutomaticUpdate: v1alpha1.NexusAutomaticUpdate{Disabled: true}, ServiceAccountName: "nexus-test", - Resources: validation.DefaultResources, - Image: validation.NexusCommunityImage, - LivenessProbe: validation.DefaultProbe.DeepCopy(), - ReadinessProbe: validation.DefaultProbe.DeepCopy(), + Resources: v1alpha1.DefaultResources, + Image: v1alpha1.NexusCommunityImage, + LivenessProbe: v1alpha1.DefaultProbe.DeepCopy(), + ReadinessProbe: v1alpha1.DefaultProbe.DeepCopy(), }, } ) @@ -341,12 +340,12 @@ func Test_equalPullPolicies(t *testing.T) { // now let's set the latest tag on the images so we can test for the PullAlways pull policy in that scenario as well depDeployment.Spec.Template.Spec.Containers[0].ImagePullPolicy = corev1.PullAlways - reqDeployment.Spec.Template.Spec.Containers[0].Image = fmt.Sprintf("%s:%s", validation.NexusCommunityImage, "latest") + reqDeployment.Spec.Template.Spec.Containers[0].Image = fmt.Sprintf("%s:%s", v1alpha1.NexusCommunityImage, "latest") assert.True(t, equalPullPolicies(depDeployment, reqDeployment)) // now with an actual tag and empty pullPolicy on the required deployment - reqDeployment.Spec.Template.Spec.Containers[0].Image = fmt.Sprintf("%s:%s", validation.NexusCommunityImage, "3.25.0") - depDeployment.Spec.Template.Spec.Containers[0].Image = fmt.Sprintf("%s:%s", validation.NexusCommunityImage, "3.25.0") + reqDeployment.Spec.Template.Spec.Containers[0].Image = fmt.Sprintf("%s:%s", v1alpha1.NexusCommunityImage, "3.25.0") + depDeployment.Spec.Template.Spec.Containers[0].Image = fmt.Sprintf("%s:%s", v1alpha1.NexusCommunityImage, "3.25.0") depDeployment.Spec.Template.Spec.Containers[0].ImagePullPolicy = corev1.PullIfNotPresent assert.True(t, equalPullPolicies(depDeployment, reqDeployment)) diff --git a/controllers/nexus/resource/networking/manager.go b/controllers/nexus/resource/networking/manager.go index 718efe6e..75a9f88c 100644 --- a/controllers/nexus/resource/networking/manager.go +++ b/controllers/nexus/resource/networking/manager.go @@ -149,7 +149,7 @@ func (m *Manager) createIngress() resource.KubernetesResource { // GetDeployedResources returns the networking resources deployed on the cluster func (m *Manager) GetDeployedResources() ([]resource.KubernetesResource, error) { - return framework.FetchDeployedResources(m.managedObjectsRef, m.nexus, m.client) + return framework.FetchDeployedResources(m.managedObjectsRef, framework.Key(m.nexus), m.client) } // GetCustomComparator returns the custom comp function used to compare a networking resource. diff --git a/controllers/nexus/resource/persistence/pvc_test.go b/controllers/nexus/resource/persistence/pvc_test.go index cf5f68c6..9fb72cc8 100644 --- a/controllers/nexus/resource/persistence/pvc_test.go +++ b/controllers/nexus/resource/persistence/pvc_test.go @@ -23,7 +23,6 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/m88i/nexus-operator/api/v1alpha1" - "github.com/m88i/nexus-operator/controllers/nexus/resource/validation" ) func Test_newPVC_defaultValues(t *testing.T) { @@ -37,7 +36,7 @@ func Test_newPVC_defaultValues(t *testing.T) { Replicas: 1, Persistence: v1alpha1.NexusPersistence{ Persistent: true, - VolumeSize: validation.DefaultVolumeSize, + VolumeSize: v1alpha1.DefaultVolumeSize, }, }, } @@ -45,7 +44,7 @@ func Test_newPVC_defaultValues(t *testing.T) { assert.Len(t, pvc.Spec.AccessModes, 1) assert.Equal(t, corev1.ReadWriteOnce, pvc.Spec.AccessModes[0]) - assert.Equal(t, resource.MustParse(validation.DefaultVolumeSize), pvc.Spec.Resources.Requests["storage"]) + assert.Equal(t, resource.MustParse(v1alpha1.DefaultVolumeSize), pvc.Spec.Resources.Requests["storage"]) } func Test_newPVC_moreThanOneReplica(t *testing.T) { diff --git a/controllers/nexus/resource/validation/events.go b/controllers/nexus/resource/validation/events.go deleted file mode 100644 index 6081059e..00000000 --- a/controllers/nexus/resource/validation/events.go +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2020 Nexus Operator and/or its 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 validation - -import ( - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/m88i/nexus-operator/api/v1alpha1" - "github.com/m88i/nexus-operator/pkg/cluster/kubernetes" - "github.com/m88i/nexus-operator/pkg/logger" -) - -const changedNexusReason = "NexusSpecChanged" - -func createChangedNexusEvent(nexus *v1alpha1.Nexus, scheme *runtime.Scheme, c client.Client, field string) { - log := logger.GetLoggerWithResource("validation_event", nexus) - err := kubernetes.RaiseWarnEventf(nexus, scheme, c, changedNexusReason, "'%s' has been changed in %s/%s. Check the logs for more information", field, nexus.Namespace, nexus.Name) - if err != nil { - log.Error(err, "Unable to raise event for changing in Nexus CR", "field", field) - } -} diff --git a/controllers/nexus/resource/validation/events_test.go b/controllers/nexus/resource/validation/events_test.go deleted file mode 100644 index 43cbf52d..00000000 --- a/controllers/nexus/resource/validation/events_test.go +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2020 Nexus Operator and/or its 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 validation - -import ( - ctx "context" - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/m88i/nexus-operator/api/v1alpha1" - "github.com/m88i/nexus-operator/pkg/test" -) - -func Test_createChangedNexusEvent(t *testing.T) { - nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus", Namespace: "test"}} - client := test.NewFakeClientBuilder().Build() - field := "some-field" - - // first, let's test a failure - client.SetMockErrorForOneRequest(fmt.Errorf("mock err")) - createChangedNexusEvent(nexus, client.Scheme(), client, field) - eventList := &corev1.EventList{} - _ = client.List(ctx.TODO(), eventList) - assert.Len(t, eventList.Items, 0) - - // now a successful one - createChangedNexusEvent(nexus, client.Scheme(), client, field) - _ = client.List(ctx.TODO(), eventList) - assert.Len(t, eventList.Items, 1) - event := eventList.Items[0] - assert.Equal(t, changedNexusReason, event.Reason) -} diff --git a/controllers/nexus/resource/validation/validation.go b/controllers/nexus/resource/validation/validation.go deleted file mode 100644 index 0bca6525..00000000 --- a/controllers/nexus/resource/validation/validation.go +++ /dev/null @@ -1,304 +0,0 @@ -// Copyright 2020 Nexus Operator and/or its 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 validation - -import ( - "fmt" - "strings" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/m88i/nexus-operator/api/v1alpha1" - "github.com/m88i/nexus-operator/controllers/nexus/update" - "github.com/m88i/nexus-operator/pkg/cluster/discovery" - "github.com/m88i/nexus-operator/pkg/logger" -) - -const ( - discOCPFailureFormat = "unable to determine if cluster is Openshift: %v" - discFailureFormat = "unable to determine if %s are available: %v" // resource type, error - unspecifiedExposeAsFormat = "'spec.exposeAs' left unspecified, setting to: " -) - -type Validator struct { - client client.Client - scheme *runtime.Scheme - log logger.Logger - routeAvailable, ingressAvailable, ocp bool -} - -// NewValidator creates a new validator to set defaults, validate and update the Nexus CR -func NewValidator(client client.Client, scheme *runtime.Scheme) (*Validator, error) { - routeAvailable, err := discovery.IsRouteAvailable() - if err != nil { - return nil, fmt.Errorf(discFailureFormat, "routes", err) - } - - ingressAvailable, err := discovery.IsIngressAvailable() - if err != nil { - return nil, fmt.Errorf(discFailureFormat, "ingresses", err) - } - - legacyIngressAvailable, err := discovery.IsLegacyIngressAvailable() - if err != nil { - return nil, fmt.Errorf(discFailureFormat, "ingresses", err) - } - - ocp, err := discovery.IsOpenShift() - if err != nil { - return nil, fmt.Errorf(discOCPFailureFormat, err) - } - - return &Validator{ - client: client, - scheme: scheme, - routeAvailable: routeAvailable, - ingressAvailable: ingressAvailable || legacyIngressAvailable, - ocp: ocp, - }, nil -} - -// SetDefaultsAndValidate returns a copy of the parameter Nexus with defaults set and an error if validation fails. -func (v *Validator) SetDefaultsAndValidate(nexus *v1alpha1.Nexus) (*v1alpha1.Nexus, error) { - v.log = logger.GetLoggerWithResource("nexus_validation", nexus) - n := v.setDefaults(nexus) - return n, v.validate(n) -} - -func (v *Validator) validate(nexus *v1alpha1.Nexus) error { - return v.validateNetworking(nexus) -} - -func (v *Validator) validateNetworking(nexus *v1alpha1.Nexus) error { - if !nexus.Spec.Networking.Expose { - v.log.Debug("'spec.networking.expose' set to 'false', ignoring networking configuration") - return nil - } - - if !v.ingressAvailable && nexus.Spec.Networking.ExposeAs == v1alpha1.IngressExposeType { - v.log.Warn("Ingresses are not available on your cluster. Make sure to be running Kubernetes > 1.14 or if you're running Openshift set ", "spec.networking.exposeAs", v1alpha1.IngressExposeType, "Also try", v1alpha1.NodePortExposeType) - return fmt.Errorf("ingress expose required, but unavailable") - } - - if !v.routeAvailable && nexus.Spec.Networking.ExposeAs == v1alpha1.RouteExposeType { - v.log.Warn("Routes are not available on your cluster. If you're running Kubernetes 1.14 or higher try setting ", "'spec.networking.exposeAs'", v1alpha1.IngressExposeType, "Also try", v1alpha1.NodePortExposeType) - return fmt.Errorf("route expose required, but unavailable") - } - - if nexus.Spec.Networking.ExposeAs == v1alpha1.NodePortExposeType && nexus.Spec.Networking.NodePort == 0 { - v.log.Warn("NodePort networking requires a port. Check the Nexus resource 'spec.networking.nodePort' parameter") - return fmt.Errorf("nodeport expose required, but no port informed") - } - - if nexus.Spec.Networking.ExposeAs == v1alpha1.IngressExposeType && len(nexus.Spec.Networking.Host) == 0 { - v.log.Warn("Ingress networking requires a host. Check the Nexus resource 'spec.networking.host' parameter") - return fmt.Errorf("ingress expose required, but no host informed") - } - - if len(nexus.Spec.Networking.TLS.SecretName) > 0 && nexus.Spec.Networking.ExposeAs != v1alpha1.IngressExposeType { - v.log.Warn("'spec.networking.tls.secretName' is only available when using an Ingress. Try setting ", "spec.networking.exposeAs'", v1alpha1.IngressExposeType) - return fmt.Errorf("tls secret name informed, but using route") - } - - if nexus.Spec.Networking.TLS.Mandatory && nexus.Spec.Networking.ExposeAs != v1alpha1.RouteExposeType { - v.log.Warn("'spec.networking.tls.mandatory' is only available when using a Route. Try setting ", "spec.networking.exposeAs'", v1alpha1.RouteExposeType) - return fmt.Errorf("tls set to mandatory, but using ingress") - } - - return nil -} - -func (v *Validator) setDefaults(nexus *v1alpha1.Nexus) *v1alpha1.Nexus { - n := nexus.DeepCopy() - v.setDeploymentDefaults(n) - v.setUpdateDefaults(n) - v.setNetworkingDefaults(n) - v.setPersistenceDefaults(n) - v.setSecurityDefaults(n) - return n -} - -func (v *Validator) setDeploymentDefaults(nexus *v1alpha1.Nexus) { - v.setReplicasDefaults(nexus) - v.setResourcesDefaults(nexus) - v.setImageDefaults(nexus) - v.setProbeDefaults(nexus) -} - -func (v *Validator) setReplicasDefaults(nexus *v1alpha1.Nexus) { - if nexus.Spec.Replicas > maxReplicas { - v.log.Warn("Number of replicas not supported", "MaxSupportedReplicas", maxReplicas, "DesiredReplicas", nexus.Spec.Replicas) - nexus.Spec.Replicas = ensureMaximum(nexus.Spec.Replicas, maxReplicas) - } -} - -func (v *Validator) setResourcesDefaults(nexus *v1alpha1.Nexus) { - if nexus.Spec.Resources.Requests == nil && nexus.Spec.Resources.Limits == nil { - nexus.Spec.Resources = DefaultResources - } -} - -func (v *Validator) setImageDefaults(nexus *v1alpha1.Nexus) { - if nexus.Spec.UseRedHatImage { - if len(nexus.Spec.Image) > 0 { - v.log.Warn("Nexus CR configured to the use Red Hat Certified Image, ignoring 'spec.image' field.") - } - nexus.Spec.Image = NexusCertifiedImage - } else if len(nexus.Spec.Image) == 0 { - nexus.Spec.Image = NexusCommunityImage - } - - if len(nexus.Spec.ImagePullPolicy) > 0 && - nexus.Spec.ImagePullPolicy != corev1.PullAlways && - nexus.Spec.ImagePullPolicy != corev1.PullIfNotPresent && - nexus.Spec.ImagePullPolicy != corev1.PullNever { - - v.log.Warn("Invalid 'spec.imagePullPolicy', unsetting the value. The pull policy will be determined by the image tag. Valid value are", "#1", corev1.PullAlways, "#2", corev1.PullIfNotPresent, "#3", corev1.PullNever) - nexus.Spec.ImagePullPolicy = "" - } -} - -func (v *Validator) setProbeDefaults(nexus *v1alpha1.Nexus) { - if nexus.Spec.LivenessProbe != nil { - nexus.Spec.LivenessProbe.FailureThreshold = - ensureMinimum(nexus.Spec.LivenessProbe.FailureThreshold, 1) - nexus.Spec.LivenessProbe.InitialDelaySeconds = - ensureMinimum(nexus.Spec.LivenessProbe.InitialDelaySeconds, 0) - nexus.Spec.LivenessProbe.PeriodSeconds = - ensureMinimum(nexus.Spec.LivenessProbe.PeriodSeconds, 1) - nexus.Spec.LivenessProbe.TimeoutSeconds = - ensureMinimum(nexus.Spec.LivenessProbe.TimeoutSeconds, 1) - } else { - nexus.Spec.LivenessProbe = DefaultProbe.DeepCopy() - } - - // SuccessThreshold for Liveness Probes must be 1 - nexus.Spec.LivenessProbe.SuccessThreshold = 1 - - if nexus.Spec.ReadinessProbe != nil { - nexus.Spec.ReadinessProbe.FailureThreshold = - ensureMinimum(nexus.Spec.ReadinessProbe.FailureThreshold, 1) - nexus.Spec.ReadinessProbe.InitialDelaySeconds = - ensureMinimum(nexus.Spec.ReadinessProbe.InitialDelaySeconds, 0) - nexus.Spec.ReadinessProbe.PeriodSeconds = - ensureMinimum(nexus.Spec.ReadinessProbe.PeriodSeconds, 1) - nexus.Spec.ReadinessProbe.SuccessThreshold = - ensureMinimum(nexus.Spec.ReadinessProbe.SuccessThreshold, 1) - nexus.Spec.ReadinessProbe.TimeoutSeconds = - ensureMinimum(nexus.Spec.ReadinessProbe.TimeoutSeconds, 1) - } else { - nexus.Spec.ReadinessProbe = DefaultProbe.DeepCopy() - } -} - -// must be called only after image defaults have been set -func (v *Validator) setUpdateDefaults(nexus *v1alpha1.Nexus) { - if nexus.Spec.AutomaticUpdate.Disabled { - return - } - - image := strings.Split(nexus.Spec.Image, ":")[0] - if image != NexusCommunityImage { - v.log.Warn("Automatic Updates are enabled, but 'spec.image' is not using the community image. Disabling automatic updates", "Community Image", NexusCommunityImage) - nexus.Spec.AutomaticUpdate.Disabled = true - return - } - - if nexus.Spec.AutomaticUpdate.MinorVersion == nil { - v.log.Debug("Automatic Updates are enabled, but no minor was informed. Fetching the most recent...") - minor, err := update.GetLatestMinor() - if err != nil { - v.log.Error(err, "Unable to fetch the most recent minor. Disabling automatic updates.") - nexus.Spec.AutomaticUpdate.Disabled = true - createChangedNexusEvent(nexus, v.scheme, v.client, "spec.automaticUpdate.disabled") - return - } - nexus.Spec.AutomaticUpdate.MinorVersion = &minor - } - - v.log.Debug("Fetching the latest micro from minor", "MinorVersion", *nexus.Spec.AutomaticUpdate.MinorVersion) - tag, ok := update.GetLatestMicro(*nexus.Spec.AutomaticUpdate.MinorVersion) - if !ok { - // the informed minor doesn't exist, let's try the latest minor - v.log.Warn("Latest tag for minor version not found. Trying the latest minor instead", "Informed tag", *nexus.Spec.AutomaticUpdate.MinorVersion) - minor, err := update.GetLatestMinor() - if err != nil { - v.log.Error(err, "Unable to fetch the most recent minor: %v. Disabling automatic updates.") - nexus.Spec.AutomaticUpdate.Disabled = true - createChangedNexusEvent(nexus, v.scheme, v.client, "spec.automaticUpdate.disabled") - return - } - v.log.Info("Setting 'spec.automaticUpdate.minorVersion to", "MinorTag", minor) - nexus.Spec.AutomaticUpdate.MinorVersion = &minor - // no need to check for the tag existence here, - // we would have gotten an error from GetLatestMinor() if it didn't - tag, _ = update.GetLatestMicro(minor) - } - newImage := fmt.Sprintf("%s:%s", image, tag) - if newImage != nexus.Spec.Image { - v.log.Debug("Replacing 'spec.image'", "OldImage", nexus.Spec.Image, "NewImage", newImage) - nexus.Spec.Image = newImage - } -} - -func (v *Validator) setNetworkingDefaults(nexus *v1alpha1.Nexus) { - if !nexus.Spec.Networking.Expose { - return - } - - if len(nexus.Spec.Networking.ExposeAs) == 0 { - if v.ocp { - v.log.Info(unspecifiedExposeAsFormat, "ExposeType", v1alpha1.RouteExposeType) - nexus.Spec.Networking.ExposeAs = v1alpha1.RouteExposeType - } else if v.ingressAvailable { - v.log.Info(unspecifiedExposeAsFormat, "ExposeType", v1alpha1.IngressExposeType) - nexus.Spec.Networking.ExposeAs = v1alpha1.IngressExposeType - } else { - // we're on kubernetes < 1.14 - // try setting nodePort, validation will catch it if impossible - v.log.Info("On Kubernetes, but Ingresses are not available") - v.log.Info(unspecifiedExposeAsFormat, "ExposeType", v1alpha1.NodePortExposeType) - nexus.Spec.Networking.ExposeAs = v1alpha1.NodePortExposeType - } - } -} - -func (v *Validator) setPersistenceDefaults(nexus *v1alpha1.Nexus) { - if nexus.Spec.Persistence.Persistent && len(nexus.Spec.Persistence.VolumeSize) == 0 { - nexus.Spec.Persistence.VolumeSize = DefaultVolumeSize - } -} - -func (v *Validator) setSecurityDefaults(nexus *v1alpha1.Nexus) { - if len(nexus.Spec.ServiceAccountName) == 0 { - nexus.Spec.ServiceAccountName = nexus.Name - } -} - -func ensureMinimum(value, minimum int32) int32 { - if value < minimum { - return minimum - } - return value -} - -func ensureMaximum(value, max int32) int32 { - if value > max { - return max - } - return value -} diff --git a/controllers/nexus/update/log.go b/controllers/nexus/update/log.go index 2ce090b5..eaed0141 100644 --- a/controllers/nexus/update/log.go +++ b/controllers/nexus/update/log.go @@ -17,8 +17,7 @@ package update import "github.com/m88i/nexus-operator/pkg/logger" const ( - defaultLogName = "update" monitorLogName = "update_monitor" ) -var log = logger.GetLogger(defaultLogName) +var log = logger.GetLogger(monitorLogName) diff --git a/controllers/nexus/update/monitor.go b/controllers/nexus/update/monitor.go index 5bfaf1e6..b7978976 100644 --- a/controllers/nexus/update/monitor.go +++ b/controllers/nexus/update/monitor.go @@ -25,6 +25,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/m88i/nexus-operator/api/v1alpha1" + "github.com/m88i/nexus-operator/pkg/framework" "github.com/m88i/nexus-operator/pkg/logger" ) @@ -57,7 +58,6 @@ func HandleUpdate(nexus *v1alpha1.Nexus, deployed, required *appsv1.Deployment, } log = logger.GetLoggerWithResource(monitorLogName, nexus) - defer func() { log = logger.GetLogger(defaultLogName) }() // it's important to check if this is a new update before checking ongoing updates because // if this is a new update, the one that was happening before no longer matters @@ -127,7 +127,7 @@ func isNewUpdate(deployed, required *appsv1.Deployment) (updating bool, previous deployedImageParts := strings.Split(depImage, ":") requiredImageParts := strings.Split(reqImage, ":") - updating, err := HigherVersion(requiredImageParts[1], deployedImageParts[1]) + updating, err := framework.HigherVersion(requiredImageParts[1], deployedImageParts[1]) if err != nil { log.Error(err, "Unable to check if the required Deployment is an update when comparing to the deployed one", "deployment", required.Name) return @@ -159,9 +159,9 @@ func differentImagesOrMinors(deployed, required *appsv1.Deployment) bool { } // we should be able to assume there will be no parsing error, we just created this deployment in the reconcile loop - reqMinor, _ := getMinor(requiredImageParts[1]) + reqMinor, _ := framework.GetMinor(requiredImageParts[1]) // the deployed one, on the other hand, might have been tampered with - depMinor, err := getMinor(deployedImageParts[1]) + depMinor, err := framework.GetMinor(deployedImageParts[1]) if err != nil { log.Error(err, "Unable to parse the deployed Deployment's tag. Cannot determine if this is an update. Has it been tampered with?", "deployment", deployed.Name) return true diff --git a/controllers/nexus_controller.go b/controllers/nexus_controller.go index 0929bd21..c4507c56 100644 --- a/controllers/nexus_controller.go +++ b/controllers/nexus_controller.go @@ -43,8 +43,6 @@ import ( "github.com/m88i/nexus-operator/pkg/cluster/kubernetes" "github.com/m88i/nexus-operator/pkg/cluster/openshift" "github.com/m88i/nexus-operator/pkg/framework" - - "github.com/m88i/nexus-operator/controllers/nexus/resource/validation" ) const ( @@ -81,8 +79,8 @@ func (r *NexusReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { result := ctrl.Result{} // Fetch the Nexus instance - instance := &appsv1alpha1.Nexus{} - err := r.Get(context.TODO(), req.NamespacedName, instance) + nexus := &appsv1alpha1.Nexus{} + err := r.Get(context.TODO(), req.NamespacedName, nexus) if err != nil { if errors.IsNotFound(err) { // Request object not found, could have been deleted after reconcile request. @@ -94,21 +92,11 @@ func (r *NexusReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return result, err } - v, err := validation.NewValidator(r, r.Scheme) - if err != nil { - // Error using the discovery API - requeue the request. - return result, err - } - - validatedNexus, err := v.SetDefaultsAndValidate(instance) // In case of any errors from here, we should update the Nexus CR and its status - defer r.updateNexus(validatedNexus, instance, &err) - if err != nil { - return result, err - } + defer r.updateNexus(nexus, &err) // Initialize the resource managers - err = r.Supervisor.InitManagers(validatedNexus) + err = r.Supervisor.InitManagers(nexus) if err != nil { return result, err } @@ -129,7 +117,7 @@ func (r *NexusReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } deltas := comparator.Compare(deployedRes, requiredRes) - writer := write.New(r).WithOwnerController(validatedNexus, r.Scheme) + writer := write.New(r).WithOwnerController(nexus, r.Scheme) for resourceType, delta := range deltas { if !delta.HasChanges() { continue @@ -153,12 +141,12 @@ func (r *NexusReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } } - if err = r.ensureServerUpdates(validatedNexus); err != nil { + if err = r.ensureServerUpdates(nexus); err != nil { return result, err } // Check if we are performing an update and act upon it if needed - err = r.handleUpdate(validatedNexus, requiredRes, deployedRes) + err = r.handleUpdate(nexus, requiredRes, deployedRes) return result, err } @@ -204,8 +192,9 @@ func (r *NexusReconciler) ensureServerUpdates(instance *appsv1alpha1.Nexus) erro return nil } -func (r *NexusReconciler) updateNexus(nexus *appsv1alpha1.Nexus, originalNexus *appsv1alpha1.Nexus, err *error) { +func (r *NexusReconciler) updateNexus(nexus *appsv1alpha1.Nexus, err *error) { r.Log.Info("Updating application status before leaving") + originalNexus := nexus.DeepCopy() if statusErr := r.getNexusDeploymentStatus(nexus); statusErr != nil { r.Log.Error(*err, "Error while fetching Nexus Deployment status") @@ -227,31 +216,6 @@ func (r *NexusReconciler) updateNexus(nexus *appsv1alpha1.Nexus, originalNexus * r.Log.Error(urlErr, "Error while fetching Nexus URL status") } - if !reflect.DeepEqual(originalNexus.Spec, nexus.Spec) { - r.Log.Info("Updating Nexus instance ", "Nexus instance", nexus.Name) - waitErr := wait.Poll(updatePollWaitTimeout, updateCancelTimeout, func() (bool, error) { - if updateErr := r.Update(context.TODO(), nexus); errors.IsConflict(updateErr) { - newNexus := &appsv1alpha1.Nexus{ObjectMeta: v1.ObjectMeta{ - Name: nexus.Name, - Namespace: nexus.Namespace, - }} - if err := r.Get(context.TODO(), framework.Key(newNexus), newNexus); err != nil { - return false, err - } - // we override only the spec, which we are interested into - newNexus.Spec = nexus.Spec - nexus = newNexus - return false, nil - } else if updateErr != nil { - return false, updateErr - } - return true, nil - }) - if waitErr != nil { - r.Log.Error(waitErr, "Error while updating Nexus status") - } - } - if !reflect.DeepEqual(originalNexus.Status, nexus.Status) { r.Log.Info("Updating status for ", "Nexus instance", nexus.Name) waitErr := wait.Poll(updatePollWaitTimeout, updateCancelTimeout, func() (bool, error) { @@ -263,7 +227,7 @@ func (r *NexusReconciler) updateNexus(nexus *appsv1alpha1.Nexus, originalNexus * if err := r.Get(context.TODO(), framework.Key(newNexus), newNexus); err != nil { return false, err } - // we override only the spec, which we are interested into + // we override only the status, which we are interested into newNexus.Status = nexus.Status nexus = newNexus return false, nil diff --git a/go.sum b/go.sum index 7c74d616..47981366 100644 --- a/go.sum +++ b/go.sum @@ -906,6 +906,7 @@ k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8 k8s.io/gengo v0.0.0-20190822140433-26a664648505/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/gengo v0.0.0-20200114144118-36b2048a9120/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= +k8s.io/gengo v0.0.0-20200428234225-8167cfdcfc14 h1:t4L10Qfx/p7ASH3gXCdIUtPbbIuegCoUJf3TMSFekjw= k8s.io/gengo v0.0.0-20200428234225-8167cfdcfc14/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/klog v0.0.0-20181102134211-b9b56d5dfc92/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= k8s.io/klog v0.3.0/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= diff --git a/main.go b/main.go index 8bacc0f7..2c20a7cf 100644 --- a/main.go +++ b/main.go @@ -31,6 +31,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/log/zap" + appsm88iiov1alpha1 "github.com/m88i/nexus-operator/api/v1alpha1" appsv1alpha1 "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/controllers" "github.com/m88i/nexus-operator/controllers/nexus/resource" @@ -48,6 +49,7 @@ func init() { utilruntime.Must(routev1.Install(scheme)) utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(appsv1alpha1.AddToScheme(scheme)) + utilruntime.Must(appsm88iiov1alpha1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme } @@ -97,6 +99,10 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "Nexus") os.Exit(1) } + if err = (&appsm88iiov1alpha1.Nexus{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "Nexus") + os.Exit(1) + } // +kubebuilder:scaffold:builder setupLog.Info("starting manager") diff --git a/pkg/framework/fetcher.go b/pkg/framework/fetcher.go index 87c8b45b..95f6313a 100644 --- a/pkg/framework/fetcher.go +++ b/pkg/framework/fetcher.go @@ -22,19 +22,16 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/m88i/nexus-operator/api/v1alpha1" ) // FetchDeployedResources fetches deployed resources whose Kind is present in "managedObjectsRef" -func FetchDeployedResources(managedObjectsRef map[string]resource.KubernetesResource, nexus *v1alpha1.Nexus, cli client.Client) ([]resource.KubernetesResource, error) { +func FetchDeployedResources(managedObjectsRef map[string]resource.KubernetesResource, key types.NamespacedName, cli client.Client) ([]resource.KubernetesResource, error) { var resources []resource.KubernetesResource for resKind, resRef := range managedObjectsRef { - key := Key(nexus) if err := Fetch(cli, key, resRef, resKind); err == nil { resources = append(resources, resRef) } else if !errors.IsNotFound(err) { - return nil, fmt.Errorf("could not fetch %s (%s/%s): %v", resKind, nexus.Namespace, nexus.Name, err) + return nil, fmt.Errorf("could not fetch %s (%s): %v", resKind, key.String(), err) } else { log.Debug("Unable to find resource", "kind", resKind, "namespacedName", key) } diff --git a/pkg/framework/fetcher_test.go b/pkg/framework/fetcher_test.go index f5fe3e90..d3343aaf 100644 --- a/pkg/framework/fetcher_test.go +++ b/pkg/framework/fetcher_test.go @@ -42,7 +42,7 @@ func TestFetchDeployedResources(t *testing.T) { } cli := test.NewFakeClientBuilder(deployment, service).Build() - gotResources, err := FetchDeployedResources(managedObjectsRef, nexus, cli) + gotResources, err := FetchDeployedResources(managedObjectsRef, Key(nexus), cli) assert.Nil(t, err) assert.Len(t, gotResources, 2) @@ -56,7 +56,7 @@ func TestFetchDeployedResourcesFailure(t *testing.T) { mockErrorMsg := "mock error" cli.SetMockError(goerrors.New(mockErrorMsg)) - _, err := FetchDeployedResources(managedObjectsRef, nexus, cli) + _, err := FetchDeployedResources(managedObjectsRef, Key(nexus), cli) assert.Contains(t, err.Error(), mockErrorMsg) } diff --git a/controllers/nexus/update/tags.go b/pkg/framework/tags.go similarity index 96% rename from controllers/nexus/update/tags.go rename to pkg/framework/tags.go index 11878186..e0e495d9 100644 --- a/controllers/nexus/update/tags.go +++ b/pkg/framework/tags.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package update +package framework import ( "fmt" @@ -43,11 +43,11 @@ var ( // HigherVersion checks if thisTag is of a higher version than otherTag func HigherVersion(thisTag, otherTag string) (bool, error) { - thisMinor, err := getMinor(thisTag) + thisMinor, err := GetMinor(thisTag) if err != nil { return false, fmt.Errorf(tagParseFailureFormat, thisTag, err) } - otherMinor, err := getMinor(otherTag) + otherMinor, err := GetMinor(otherTag) if err != nil { return false, fmt.Errorf(tagParseFailureFormat, otherTag, err) } @@ -137,7 +137,7 @@ func getTags() ([]string, error) { func parseTagsAndUpdate(tags []string) error { for _, candidateTag := range tags { if candidateTag != "latest" { - candidateMinor, err := getMinor(candidateTag) + candidateMinor, err := GetMinor(candidateTag) if err != nil { return fmt.Errorf(tagParseFailureFormat, candidateTag, err) } @@ -160,7 +160,7 @@ func parseTagsAndUpdate(tags []string) error { return nil } -func getMinor(tag string) (int, error) { +func GetMinor(tag string) (int, error) { return strconv.Atoi(strings.Split(tag, ".")[1]) } diff --git a/controllers/nexus/update/tags_test.go b/pkg/framework/tags_test.go similarity index 97% rename from controllers/nexus/update/tags_test.go rename to pkg/framework/tags_test.go index eff517d1..c60759e0 100644 --- a/controllers/nexus/update/tags_test.go +++ b/pkg/framework/tags_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package update +package framework import ( "testing" @@ -151,11 +151,11 @@ func TestGetMinor(t *testing.T) { validTag := "3.25.0" invalidTag := "3..0" - minor, err := getMinor(validTag) + minor, err := GetMinor(validTag) assert.Nil(t, err) assert.Equal(t, 25, minor) - _, err = getMinor(invalidTag) + _, err = GetMinor(invalidTag) assert.NotNil(t, err) } diff --git a/pkg/test/client.go b/pkg/test/client.go index e1632c66..cf0f2a7c 100644 --- a/pkg/test/client.go +++ b/pkg/test/client.go @@ -41,6 +41,10 @@ const ( openshiftGroupVersion = "openshift.io/v1" ) +// TODO: break this in FakeClient and FakeDiscovery +// TODO: move FakeClient and the actual client to a separate package +// TODO: move FakeDiscovery to the discovery package + // FakeClientBuilder allows building a FakeClient according to // the desired cluster capabilities type FakeClientBuilder struct { From 28842588501b5e06a5a1223e2834c823743ef729 Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Thu, 17 Dec 2020 15:07:34 -0300 Subject: [PATCH 02/10] Break FakeClient into FakeClient and FakeDisc Improves cohesion for the FakeClient/FakeDisc and breaks an import cycle created due to admission testing needing a fake discovery implementation and the previous implementation also importing v1alpha1. Signed-off-by: Lucas Caparelli --- api/v1alpha1/nexus_webhook.go | 24 +- api/v1alpha1/validation_test.go | 1 - main.go | 2 - pkg/cluster/discovery/discovery_test.go | 4 +- pkg/cluster/discovery/fake.go | 172 ++++++++++++ pkg/cluster/discovery/fake_test.go | 256 ++++++++++++++++++ pkg/cluster/discovery/kubernetes_test.go | 10 +- pkg/cluster/discovery/openshift_test.go | 10 +- pkg/framework/kind/kinds.go | 1 + pkg/test/client.go | 201 +------------- pkg/test/client_test.go | 320 ++--------------------- pkg/test/utils.go | 8 + pkg/test/utils_test.go | 27 ++ pkg/util/error.go | 22 -- 14 files changed, 515 insertions(+), 543 deletions(-) create mode 100644 pkg/cluster/discovery/fake.go create mode 100644 pkg/cluster/discovery/fake_test.go delete mode 100644 pkg/util/error.go diff --git a/api/v1alpha1/nexus_webhook.go b/api/v1alpha1/nexus_webhook.go index 5136eb97..2316b8f2 100644 --- a/api/v1alpha1/nexus_webhook.go +++ b/api/v1alpha1/nexus_webhook.go @@ -27,9 +27,9 @@ const admissionLogName = "admission" // log is for logging in this package. var log = logger.GetLogger(admissionLogName) -func (r *Nexus) SetupWebhookWithManager(mgr ctrl.Manager) error { +func (n *Nexus) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). - For(r). + For(n). Complete() } @@ -40,12 +40,12 @@ func (r *Nexus) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Defaulter = &Nexus{} // Default implements webhook.Defaulter so a webhook will be registered for the type -func (r *Nexus) Default() { - log = logger.GetLoggerWithResource(admissionLogName, r) +func (n *Nexus) Default() { + log = logger.GetLoggerWithResource(admissionLogName, n) defer func() { log = logger.GetLogger(admissionLogName) }() log.Info("Setting defaults and making necessary changes to the Nexus") - newMutator(r).mutate() + newMutator(n).mutate() } // if we ever need validation upon delete requests change verbs to "verbs=create;update;delete". @@ -54,25 +54,25 @@ func (r *Nexus) Default() { var _ webhook.Validator = &Nexus{} // ValidateCreate implements webhook.Validator so a webhook will be registered for the type -func (r *Nexus) ValidateCreate() error { - log = logger.GetLoggerWithResource(admissionLogName, r) +func (n *Nexus) ValidateCreate() error { + log = logger.GetLoggerWithResource(admissionLogName, n) defer func() { log = logger.GetLogger(admissionLogName) }() log.Info("Validating a new Nexus") - return newValidator(r).validate() + return newValidator(n).validate() } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type -func (r *Nexus) ValidateUpdate(old runtime.Object) error { - log = logger.GetLoggerWithResource(admissionLogName, r) +func (n *Nexus) ValidateUpdate(old runtime.Object) error { + log = logger.GetLoggerWithResource(admissionLogName, n) defer func() { log = logger.GetLogger(admissionLogName) }() log.Info("Validating an update to a existing Nexus") - return newValidator(r).validate() + return newValidator(n).validate() } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type -func (r *Nexus) ValidateDelete() error { +func (n *Nexus) ValidateDelete() error { // we don't care about validation on delete requests return nil } diff --git a/api/v1alpha1/validation_test.go b/api/v1alpha1/validation_test.go index e49b00ca..f546ddb7 100644 --- a/api/v1alpha1/validation_test.go +++ b/api/v1alpha1/validation_test.go @@ -25,7 +25,6 @@ import ( "github.com/m88i/nexus-operator/pkg/cluster/discovery" "github.com/m88i/nexus-operator/pkg/framework" "github.com/m88i/nexus-operator/pkg/logger" - "github.com/m88i/nexus-operator/pkg/test" ) // TODO: fix the tests diff --git a/main.go b/main.go index 2c20a7cf..59f7eedc 100644 --- a/main.go +++ b/main.go @@ -32,7 +32,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" appsm88iiov1alpha1 "github.com/m88i/nexus-operator/api/v1alpha1" - appsv1alpha1 "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/controllers" "github.com/m88i/nexus-operator/controllers/nexus/resource" "github.com/m88i/nexus-operator/pkg/cluster/discovery" @@ -48,7 +47,6 @@ func init() { // adding routev1 utilruntime.Must(routev1.Install(scheme)) utilruntime.Must(clientgoscheme.AddToScheme(scheme)) - utilruntime.Must(appsv1alpha1.AddToScheme(scheme)) utilruntime.Must(appsm88iiov1alpha1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme } diff --git a/pkg/cluster/discovery/discovery_test.go b/pkg/cluster/discovery/discovery_test.go index 0023db04..471fc427 100644 --- a/pkg/cluster/discovery/discovery_test.go +++ b/pkg/cluster/discovery/discovery_test.go @@ -18,12 +18,10 @@ import ( "testing" "github.com/stretchr/testify/assert" - - "github.com/m88i/nexus-operator/pkg/test" ) func TestSetDiscClient(t *testing.T) { - disc := test.NewFakeClientBuilder().Build() + disc := NewFakeDiscBuilder().Build() SetClient(disc) assert.Equal(t, disc, cli) } diff --git a/pkg/cluster/discovery/fake.go b/pkg/cluster/discovery/fake.go new file mode 100644 index 00000000..db9f5ad0 --- /dev/null +++ b/pkg/cluster/discovery/fake.go @@ -0,0 +1,172 @@ +package discovery + +import ( + openapi_v2 "github.com/googleapis/gnostic/openapiv2" + routev1 "github.com/openshift/api/route/v1" + networkingv1 "k8s.io/api/networking/v1" + networkingv1beta1 "k8s.io/api/networking/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/version" + discfake "k8s.io/client-go/discovery/fake" + "k8s.io/client-go/rest" + clienttesting "k8s.io/client-go/testing" + + "github.com/m88i/nexus-operator/pkg/framework/kind" +) + +const openshiftGroupVersion = "openshift.io/v1" + +// need to mirror this here to avoid import cycles +var m88iGroupVersion = schema.GroupVersion{Group: "apps.m88i.io", Version: "v1alpha1"} + +type FakeDiscBuilder struct { + resources []*metav1.APIResourceList +} + +// NewFakeDiscBuilder returns a builder for generating a new FakeDisc +func NewFakeDiscBuilder() *FakeDiscBuilder { + return &FakeDiscBuilder{resources: []*metav1.APIResourceList{{GroupVersion: m88iGroupVersion.String(), APIResources: []metav1.APIResource{{Kind: kind.NexusKind}}}}} +} + +// OnOpenshift makes the fake client aware of resources from Openshift +func (b *FakeDiscBuilder) OnOpenshift() *FakeDiscBuilder { + b.resources = append(b.resources, + &metav1.APIResourceList{GroupVersion: openshiftGroupVersion}, + &metav1.APIResourceList{GroupVersion: routev1.GroupVersion.String(), APIResources: []metav1.APIResource{{Kind: kind.RouteKind}}}) + return b +} + +// WithIngress makes the fake disc aware of v1 Ingresses +func (b *FakeDiscBuilder) WithIngress() *FakeDiscBuilder { + b.resources = append(b.resources, &metav1.APIResourceList{GroupVersion: networkingv1.SchemeGroupVersion.String(), APIResources: []metav1.APIResource{{Kind: kind.IngressKind}}}) + return b +} + +// WithLegacyIngress makes the fake disc aware of v1beta1 Ingresses +func (b *FakeDiscBuilder) WithLegacyIngress() *FakeDiscBuilder { + b.resources = append(b.resources, &metav1.APIResourceList{GroupVersion: networkingv1beta1.SchemeGroupVersion.String(), APIResources: []metav1.APIResource{{Kind: kind.IngressKind}}}) + return b +} + +func (b *FakeDiscBuilder) Build() *FakeDisc { + return &FakeDisc{ + disc: &discfake.FakeDiscovery{ + Fake: &clienttesting.Fake{ + Resources: b.resources, + }, + }, + } +} + +// FakeDisc wraps a fake discovery to permit mocking errors +type FakeDisc struct { + disc *discfake.FakeDiscovery + mockErr error + shouldClearError bool +} + +// SetMockError sets the error which should be returned for the following requests +// This error will continue to be served until cleared with d.ClearMockError() +func (d *FakeDisc) SetMockError(err error) { + d.shouldClearError = false + d.mockErr = err +} + +// SetMockErrorForOneRequest sets the error which should be returned for the following request +// this error will be set to nil after the next request +func (d *FakeDisc) SetMockErrorForOneRequest(err error) { + d.shouldClearError = true + d.mockErr = err +} + +// ClearMockError unsets any mock errors previously set +func (d *FakeDisc) ClearMockError() { + d.shouldClearError = false + d.mockErr = nil +} + +func (d FakeDisc) RESTClient() rest.Interface { + return d.disc.RESTClient() +} + +func (d *FakeDisc) ServerGroups() (*metav1.APIGroupList, error) { + if d.mockErr != nil { + if d.shouldClearError { + defer d.ClearMockError() + } + return nil, d.mockErr + } + return d.disc.ServerGroups() +} + +func (d *FakeDisc) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { + if d.mockErr != nil { + if d.shouldClearError { + defer d.ClearMockError() + } + return nil, d.mockErr + } + return d.disc.ServerResourcesForGroupVersion(groupVersion) +} + +// Deprecated: use ServerGroupsAndResources instead. +func (d *FakeDisc) ServerResources() ([]*metav1.APIResourceList, error) { + if d.mockErr != nil { + if d.shouldClearError { + defer d.ClearMockError() + } + return nil, d.mockErr + } + return d.disc.ServerResources() +} + +func (d *FakeDisc) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { + if d.mockErr != nil { + if d.shouldClearError { + defer d.ClearMockError() + } + return nil, nil, d.mockErr + } + return d.disc.ServerGroupsAndResources() +} + +func (d *FakeDisc) ServerPreferredResources() ([]*metav1.APIResourceList, error) { + if d.mockErr != nil { + if d.shouldClearError { + defer d.ClearMockError() + } + return nil, d.mockErr + } + return d.disc.ServerPreferredResources() +} + +func (d *FakeDisc) ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) { + if d.mockErr != nil { + if d.shouldClearError { + defer d.ClearMockError() + } + return nil, d.mockErr + } + return d.disc.ServerPreferredNamespacedResources() +} + +func (d *FakeDisc) ServerVersion() (*version.Info, error) { + if d.mockErr != nil { + if d.shouldClearError { + defer d.ClearMockError() + } + return nil, d.mockErr + } + return d.disc.ServerVersion() +} + +func (d *FakeDisc) OpenAPISchema() (*openapi_v2.Document, error) { + if d.mockErr != nil { + if d.shouldClearError { + defer d.ClearMockError() + } + return nil, d.mockErr + } + return d.disc.OpenAPISchema() +} diff --git a/pkg/cluster/discovery/fake_test.go b/pkg/cluster/discovery/fake_test.go new file mode 100644 index 00000000..74eae3be --- /dev/null +++ b/pkg/cluster/discovery/fake_test.go @@ -0,0 +1,256 @@ +package discovery + +import ( + "fmt" + "testing" + + routev1 "github.com/openshift/api/route/v1" + "github.com/stretchr/testify/assert" + networkingv1 "k8s.io/api/networking/v1" + networkingv1beta1 "k8s.io/api/networking/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/m88i/nexus-operator/pkg/framework/kind" +) + +const testErrorMsg = "test" + +func TestNewFakeDiscBuilder(t *testing.T) { + b := NewFakeDiscBuilder() + assert.True(t, resourceListsContainsGroupVersionKind(b.resources, m88iGroupVersion.String(), kind.NexusKind)) +} + +func TestFakeClientBuilder_OnOpenshift(t *testing.T) { + b := NewFakeDiscBuilder().OnOpenshift() + assert.True(t, resourceListsContainsGroupVersionKind(b.resources, routev1.GroupVersion.String(), kind.RouteKind)) + assert.True(t, resourceListsContainsGroupVersion(b.resources, openshiftGroupVersion)) +} + +func TestFakeClientBuilder_WithIngress(t *testing.T) { + b := NewFakeDiscBuilder().WithIngress() + assert.True(t, resourceListsContainsGroupVersionKind(b.resources, networkingv1.SchemeGroupVersion.String(), kind.IngressKind)) +} + +func TestFakeClientBuilder_WithLegacyIngress(t *testing.T) { + b := NewFakeDiscBuilder().WithLegacyIngress() + assert.True(t, resourceListsContainsGroupVersionKind(b.resources, networkingv1beta1.SchemeGroupVersion.String(), kind.IngressKind)) +} + +func TestFakeClientBuilder_Build(t *testing.T) { + // base + SetClient(NewFakeDiscBuilder().Build()) + ocp, _ := IsOpenShift() + assert.False(t, ocp) + withRoute, _ := IsRouteAvailable() + assert.False(t, withRoute) + withIngress, _ := IsIngressAvailable() + assert.False(t, withIngress) + withLegacyIngress, _ := IsLegacyIngressAvailable() + assert.False(t, withLegacyIngress) + + // on Openshift + SetClient(NewFakeDiscBuilder().OnOpenshift().Build()) + ocp, _ = IsOpenShift() + assert.True(t, ocp) + withRoute, _ = IsRouteAvailable() + assert.True(t, withRoute) + withIngress, _ = IsIngressAvailable() + assert.False(t, withIngress) + withLegacyIngress, _ = IsLegacyIngressAvailable() + assert.False(t, withLegacyIngress) + + // with Ingress + SetClient(NewFakeDiscBuilder().WithIngress().Build()) + ocp, _ = IsOpenShift() + assert.False(t, ocp) + withRoute, _ = IsRouteAvailable() + assert.False(t, withRoute) + withIngress, _ = IsIngressAvailable() + assert.True(t, withIngress) + withLegacyIngress, _ = IsLegacyIngressAvailable() + assert.False(t, withLegacyIngress) + + // with v1beta1 Ingress + SetClient(NewFakeDiscBuilder().WithLegacyIngress().Build()) + ocp, _ = IsOpenShift() + assert.False(t, ocp) + withRoute, _ = IsRouteAvailable() + assert.False(t, withRoute) + withIngress, _ = IsIngressAvailable() + assert.False(t, withIngress) + withLegacyIngress, _ = IsLegacyIngressAvailable() + assert.True(t, withLegacyIngress) +} + +func resourceListsContainsGroupVersion(lists []*metav1.APIResourceList, gv string) bool { + for _, list := range lists { + if list.GroupVersion == gv { + return true + } + } + return false +} + +func resourceListsContainsGroupVersionKind(lists []*metav1.APIResourceList, gv, kind string) bool { + for _, list := range lists { + if list.GroupVersion == gv { + for _, res := range list.APIResources { + if res.Kind == kind { + return true + } + } + // correct group, incorrect kind + return false + } + } + return false +} + +func TestFakeClient_SetMockError(t *testing.T) { + c := NewFakeDiscBuilder().Build() + assert.Nil(t, c.mockErr) + assert.False(t, c.shouldClearError) + c.SetMockError(fmt.Errorf(testErrorMsg)) + assert.Equal(t, c.mockErr.Error(), testErrorMsg) + assert.False(t, c.shouldClearError) +} + +func TestFakeClient_SetMockErrorForOneRequest(t *testing.T) { + c := NewFakeDiscBuilder().Build() + assert.Nil(t, c.mockErr) + assert.False(t, c.shouldClearError) + c.SetMockErrorForOneRequest(fmt.Errorf(testErrorMsg)) + assert.Equal(t, c.mockErr.Error(), testErrorMsg) + assert.True(t, c.shouldClearError) +} + +func TestFakeClient_ClearMockError(t *testing.T) { + c := NewFakeDiscBuilder().Build() + assert.Nil(t, c.mockErr) + assert.False(t, c.shouldClearError) + c.SetMockErrorForOneRequest(fmt.Errorf(testErrorMsg)) + assert.Equal(t, c.mockErr.Error(), testErrorMsg) + assert.True(t, c.shouldClearError) + c.ClearMockError() + assert.Nil(t, c.mockErr) + assert.False(t, c.shouldClearError) +} + +func TestFakeClient_RESTClient(t *testing.T) { + c := NewFakeDiscBuilder().Build() + assert.Equal(t, c.disc.RESTClient(), c.RESTClient()) +} + +func TestFakeClient_ServerGroups(t *testing.T) { + c := NewFakeDiscBuilder().Build() + mockErr := fmt.Errorf(testErrorMsg) + c.SetMockErrorForOneRequest(mockErr) + _, err := c.ServerGroups() + assert.Equal(t, mockErr, err) + + want, wantErr := c.disc.ServerGroups() + got, gotErr := c.ServerGroups() + assert.Equal(t, want, got) + assert.Equal(t, wantErr, gotErr) + assert.NotEqual(t, gotErr, mockErr) +} + +func TestFakeClient_ServerResourcesForGroupVersion(t *testing.T) { + c := NewFakeDiscBuilder().Build() + mockErr := fmt.Errorf(testErrorMsg) + c.SetMockErrorForOneRequest(mockErr) + _, err := c.ServerResourcesForGroupVersion("") + assert.Equal(t, mockErr, err) + + want, wantErr := c.disc.ServerResourcesForGroupVersion("") + got, gotErr := c.ServerResourcesForGroupVersion("") + assert.Equal(t, want, got) + assert.Equal(t, wantErr, gotErr) + assert.NotEqual(t, gotErr, mockErr) +} + +// Deprecated: use ServerGroupsAndResources instead. +func TestFakeClient_ServerResources(t *testing.T) { + c := NewFakeDiscBuilder().Build() + mockErr := fmt.Errorf(testErrorMsg) + c.SetMockErrorForOneRequest(mockErr) + _, err := c.ServerResources() + assert.Equal(t, mockErr, err) + + want, wantErr := c.disc.ServerResources() + got, gotErr := c.ServerResources() + assert.Equal(t, want, got) + assert.Equal(t, wantErr, gotErr) + assert.NotEqual(t, gotErr, mockErr) +} + +func TestFakeClient_ServerGroupsAndResources(t *testing.T) { + c := NewFakeDiscBuilder().Build() + mockErr := fmt.Errorf(testErrorMsg) + c.SetMockErrorForOneRequest(mockErr) + _, _, err := c.ServerGroupsAndResources() + assert.Equal(t, mockErr, err) + + want1, want2, wantErr := c.disc.ServerGroupsAndResources() + got1, got2, gotErr := c.ServerGroupsAndResources() + assert.Equal(t, want1, got1) + assert.Equal(t, want2, got2) + assert.Equal(t, wantErr, gotErr) + assert.NotEqual(t, gotErr, mockErr) +} + +func TestFakeClient_ServerPreferredResources(t *testing.T) { + c := NewFakeDiscBuilder().Build() + mockErr := fmt.Errorf(testErrorMsg) + c.SetMockErrorForOneRequest(mockErr) + _, err := c.ServerPreferredResources() + assert.Equal(t, mockErr, err) + + want, wantErr := c.disc.ServerPreferredResources() + got, gotErr := c.ServerPreferredResources() + assert.Equal(t, want, got) + assert.Equal(t, wantErr, gotErr) + assert.NotEqual(t, gotErr, mockErr) +} + +func TestFakeClient_ServerPreferredNamespacedResources(t *testing.T) { + c := NewFakeDiscBuilder().Build() + mockErr := fmt.Errorf(testErrorMsg) + c.SetMockErrorForOneRequest(mockErr) + _, err := c.ServerPreferredNamespacedResources() + assert.Equal(t, mockErr, err) + + want, wantErr := c.disc.ServerPreferredNamespacedResources() + got, gotErr := c.ServerPreferredNamespacedResources() + assert.Equal(t, want, got) + assert.Equal(t, wantErr, gotErr) + assert.NotEqual(t, gotErr, mockErr) +} + +func TestFakeClient_ServerVersion(t *testing.T) { + c := NewFakeDiscBuilder().Build() + mockErr := fmt.Errorf(testErrorMsg) + c.SetMockErrorForOneRequest(mockErr) + _, err := c.ServerVersion() + assert.Equal(t, mockErr, err) + + want, wantErr := c.disc.ServerVersion() + got, gotErr := c.ServerVersion() + assert.Equal(t, want, got) + assert.Equal(t, wantErr, gotErr) + assert.NotEqual(t, gotErr, mockErr) +} + +func TestFakeClient_OpenAPISchema(t *testing.T) { + c := NewFakeDiscBuilder().Build() + mockErr := fmt.Errorf(testErrorMsg) + c.SetMockErrorForOneRequest(mockErr) + _, err := c.OpenAPISchema() + assert.Equal(t, mockErr, err) + + want, wantErr := c.disc.OpenAPISchema() + got, gotErr := c.OpenAPISchema() + assert.Equal(t, want, got) + assert.Equal(t, wantErr, gotErr) + assert.NotEqual(t, gotErr, mockErr) +} diff --git a/pkg/cluster/discovery/kubernetes_test.go b/pkg/cluster/discovery/kubernetes_test.go index b67d9b68..1b3dc4d4 100644 --- a/pkg/cluster/discovery/kubernetes_test.go +++ b/pkg/cluster/discovery/kubernetes_test.go @@ -18,29 +18,27 @@ import ( "testing" "github.com/stretchr/testify/assert" - - "github.com/m88i/nexus-operator/pkg/test" ) func TestIsIngressAvailable(t *testing.T) { - cli = test.NewFakeClientBuilder().Build() + SetClient(NewFakeDiscBuilder().Build()) ingressAvailable, err := IsIngressAvailable() assert.Nil(t, err) assert.False(t, ingressAvailable) - cli = test.NewFakeClientBuilder().WithIngress().Build() + SetClient(NewFakeDiscBuilder().WithIngress().Build()) ingressAvailable, err = IsIngressAvailable() assert.Nil(t, err) assert.True(t, ingressAvailable) } func TestIsLegacyIngressAvailable(t *testing.T) { - cli = test.NewFakeClientBuilder().Build() + SetClient(NewFakeDiscBuilder().Build()) ingressAvailable, err := IsLegacyIngressAvailable() assert.Nil(t, err) assert.False(t, ingressAvailable) - cli = test.NewFakeClientBuilder().WithLegacyIngress().Build() + SetClient(NewFakeDiscBuilder().WithLegacyIngress().Build()) ingressAvailable, err = IsLegacyIngressAvailable() assert.Nil(t, err) assert.True(t, ingressAvailable) diff --git a/pkg/cluster/discovery/openshift_test.go b/pkg/cluster/discovery/openshift_test.go index 81e8da87..15d1abcd 100644 --- a/pkg/cluster/discovery/openshift_test.go +++ b/pkg/cluster/discovery/openshift_test.go @@ -18,29 +18,27 @@ import ( "testing" "github.com/stretchr/testify/assert" - - "github.com/m88i/nexus-operator/pkg/test" ) func TestIsOpenShift(t *testing.T) { - cli = test.NewFakeClientBuilder().Build() + SetClient(NewFakeDiscBuilder().Build()) isOCP, err := IsOpenShift() assert.Nil(t, err) assert.False(t, isOCP) - cli = test.NewFakeClientBuilder().OnOpenshift().Build() + SetClient(NewFakeDiscBuilder().OnOpenshift().Build()) isOCP, err = IsOpenShift() assert.Nil(t, err) assert.True(t, isOCP) } func TestIsRouteAvailable(t *testing.T) { - cli = test.NewFakeClientBuilder().Build() + SetClient(NewFakeDiscBuilder().Build()) routeAvailable, err := IsRouteAvailable() assert.Nil(t, err) assert.False(t, routeAvailable) - cli = test.NewFakeClientBuilder().OnOpenshift().Build() + SetClient(NewFakeDiscBuilder().OnOpenshift().Build()) routeAvailable, err = IsRouteAvailable() assert.Nil(t, err) assert.True(t, routeAvailable) diff --git a/pkg/framework/kind/kinds.go b/pkg/framework/kind/kinds.go index 63d95c16..8e1bf388 100644 --- a/pkg/framework/kind/kinds.go +++ b/pkg/framework/kind/kinds.go @@ -17,6 +17,7 @@ package kind const ( DeploymentKind = "Deployment" IngressKind = "Ingress" + NexusKind = "Nexus" PVCKind = "Persistent Volume Claim" RouteKind = "Route" SecretKind = "Secret" diff --git a/pkg/test/client.go b/pkg/test/client.go index cf0f2a7c..63aff009 100644 --- a/pkg/test/client.go +++ b/pkg/test/client.go @@ -17,123 +17,35 @@ package test import ( "context" - openapi_v2 "github.com/googleapis/gnostic/openapiv2" routev1 "github.com/openshift/api/route/v1" - networkingv1 "k8s.io/api/networking/v1" - networkingv1beta1 "k8s.io/api/networking/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/version" - "k8s.io/client-go/discovery" - discfake "k8s.io/client-go/discovery/fake" - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" - clienttesting "k8s.io/client-go/testing" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/m88i/nexus-operator/api/v1alpha1" - "github.com/m88i/nexus-operator/pkg/framework/kind" - "github.com/m88i/nexus-operator/pkg/util" ) -const ( - openshiftGroupVersion = "openshift.io/v1" -) - -// TODO: break this in FakeClient and FakeDiscovery -// TODO: move FakeClient and the actual client to a separate package -// TODO: move FakeDiscovery to the discovery package - -// FakeClientBuilder allows building a FakeClient according to -// the desired cluster capabilities -type FakeClientBuilder struct { - initObjs []runtime.Object - scheme *runtime.Scheme - resources []*metav1.APIResourceList -} - -// NewFakeClientBuilder will create a new fake client that is aware of minimal resource types -// and stores initObjs for initialization later -func NewFakeClientBuilder(initObjs ...runtime.Object) *FakeClientBuilder { - s := scheme.Scheme - util.Must(minimumSchemeBuilder().AddToScheme(s)) - res := []*metav1.APIResourceList{{GroupVersion: v1alpha1.GroupVersion.String()}} - - return &FakeClientBuilder{ - initObjs: initObjs, - scheme: s, - resources: res, - } -} - -// OnOpenshift makes the fake client aware of resources from Openshift -func (b *FakeClientBuilder) OnOpenshift() *FakeClientBuilder { - util.Must(schemeBuilderOnOCP().AddToScheme(b.scheme)) - b.resources = append(b.resources, - &metav1.APIResourceList{GroupVersion: openshiftGroupVersion}, - &metav1.APIResourceList{GroupVersion: routev1.GroupVersion.String(), APIResources: []metav1.APIResource{{Kind: kind.RouteKind}}}) - return b -} - -// WithIngress makes the fake client aware of v1 Ingresses -func (b *FakeClientBuilder) WithIngress() *FakeClientBuilder { - util.Must(schemeBuilderWithIngress().AddToScheme(b.scheme)) - b.resources = append(b.resources, &metav1.APIResourceList{GroupVersion: networkingv1.SchemeGroupVersion.String(), APIResources: []metav1.APIResource{{Kind: kind.IngressKind}}}) - return b -} - -// WithLegacyIngress makes the fake client aware of v1beta1 Ingresses -func (b *FakeClientBuilder) WithLegacyIngress() *FakeClientBuilder { - util.Must(schemeBuilderWithLegacyIngress().AddToScheme(b.scheme)) - b.resources = append(b.resources, &metav1.APIResourceList{GroupVersion: networkingv1beta1.SchemeGroupVersion.String(), APIResources: []metav1.APIResource{{Kind: kind.IngressKind}}}) - return b -} - -// Build returns the fake discovery client -func (b *FakeClientBuilder) Build() *FakeClient { - return &FakeClient{ - scheme: b.scheme, - client: fake.NewFakeClientWithScheme(b.scheme, b.initObjs...), - disc: &discfake.FakeDiscovery{ - Fake: &clienttesting.Fake{ - Resources: b.resources, - }, - }, - } -} - -func minimumSchemeBuilder() *runtime.SchemeBuilder { - return &runtime.SchemeBuilder{v1alpha1.SchemeBuilder.AddToScheme} -} - -func schemeBuilderOnOCP() *runtime.SchemeBuilder { - return &runtime.SchemeBuilder{routev1.Install} -} - -func schemeBuilderWithIngress() *runtime.SchemeBuilder { - return &runtime.SchemeBuilder{networkingv1.AddToScheme} -} - -func schemeBuilderWithLegacyIngress() *runtime.SchemeBuilder { - return &runtime.SchemeBuilder{networkingv1beta1.AddToScheme} -} - // FakeClient wraps an API fake client to allow mocked error responses -// Useful for covering errors other than NotFound -// It also wraps a fake discovery client -// FakeClient implements both client.Client and discovery.DiscoveryInterface type FakeClient struct { - scheme *runtime.Scheme client client.Client - disc discovery.DiscoveryInterface mockErr error shouldClearError bool } -// Scheme returns the fake client's scheme -func (c *FakeClient) Scheme() *runtime.Scheme { - return c.scheme +// NewFakeClient returns a FakeClient. +// You may initialize it with a slice of runtime.Object. +func NewFakeClient(initObjs ...runtime.Object) *FakeClient { + return &FakeClient{client: fake.NewFakeClientWithScheme(scheme(), initObjs...)} +} + +func scheme() *runtime.Scheme { + s := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(s)) + utilruntime.Must(v1alpha1.AddToScheme(s)) + utilruntime.Must(routev1.Install(s)) + return s } // SetMockError sets the error which should be returned for the following requests @@ -156,91 +68,6 @@ func (c *FakeClient) ClearMockError() { c.mockErr = nil } -func (c FakeClient) RESTClient() rest.Interface { - return c.disc.RESTClient() -} - -func (c *FakeClient) ServerGroups() (*metav1.APIGroupList, error) { - if c.mockErr != nil { - if c.shouldClearError { - defer c.ClearMockError() - } - return nil, c.mockErr - } - return c.disc.ServerGroups() -} - -func (c *FakeClient) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { - if c.mockErr != nil { - if c.shouldClearError { - defer c.ClearMockError() - } - return nil, c.mockErr - } - return c.disc.ServerResourcesForGroupVersion(groupVersion) -} - -// Deprecated: use ServerGroupsAndResources instead. -func (c *FakeClient) ServerResources() ([]*metav1.APIResourceList, error) { - if c.mockErr != nil { - if c.shouldClearError { - defer c.ClearMockError() - } - return nil, c.mockErr - } - return c.disc.ServerResources() -} - -func (c *FakeClient) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - if c.mockErr != nil { - if c.shouldClearError { - defer c.ClearMockError() - } - return nil, nil, c.mockErr - } - return c.disc.ServerGroupsAndResources() -} - -func (c *FakeClient) ServerPreferredResources() ([]*metav1.APIResourceList, error) { - if c.mockErr != nil { - if c.shouldClearError { - defer c.ClearMockError() - } - return nil, c.mockErr - } - return c.disc.ServerPreferredResources() -} - -func (c *FakeClient) ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) { - if c.mockErr != nil { - if c.shouldClearError { - defer c.ClearMockError() - } - return nil, c.mockErr - } - return c.disc.ServerPreferredNamespacedResources() -} - -func (c *FakeClient) ServerVersion() (*version.Info, error) { - if c.mockErr != nil { - if c.shouldClearError { - defer c.ClearMockError() - } - return nil, c.mockErr - } - return c.disc.ServerVersion() -} - -func (c *FakeClient) OpenAPISchema() (*openapi_v2.Document, error) { - if c.mockErr != nil { - if c.shouldClearError { - defer c.ClearMockError() - } - return nil, c.mockErr - } - return c.disc.OpenAPISchema() -} - func (c *FakeClient) Get(ctx context.Context, key client.ObjectKey, obj runtime.Object) error { if c.mockErr != nil { if c.shouldClearError { diff --git a/pkg/test/client_test.go b/pkg/test/client_test.go index 6877c559..b012848d 100644 --- a/pkg/test/client_test.go +++ b/pkg/test/client_test.go @@ -17,312 +17,29 @@ package test import ( ctx "context" "fmt" - "reflect" - "strings" "testing" - routev1 "github.com/openshift/api/route/v1" "github.com/stretchr/testify/assert" - networkingv1 "k8s.io/api/networking/v1" - networkingv1beta1 "k8s.io/api/networking/v1beta1" + appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/m88i/nexus-operator/api/v1alpha1" - "github.com/m88i/nexus-operator/pkg/cluster/discovery" + "github.com/m88i/nexus-operator/pkg/framework" ) const testErrorMsg = "test" -func TestNewFakeClientBuilder(t *testing.T) { - nexus := &v1alpha1.Nexus{} - b := NewFakeClientBuilder(nexus) - - // client.Client - assert.Len(t, b.scheme.KnownTypes(v1alpha1.GroupVersion), 10) - assert.Contains(t, b.scheme.KnownTypes(v1alpha1.GroupVersion), strings.Split(reflect.TypeOf(&v1alpha1.Nexus{}).String(), ".")[1]) - assert.Contains(t, b.scheme.KnownTypes(v1alpha1.GroupVersion), strings.Split(reflect.TypeOf(&v1alpha1.NexusList{}).String(), ".")[1]) - - // discovery.DiscoveryInterface - assert.True(t, resourceListsContainsGroupVersion(b.resources, v1alpha1.GroupVersion.String())) - - // initObjs - assert.Len(t, b.initObjs, 1) - assert.Contains(t, b.initObjs, nexus) -} - -func TestFakeClientBuilder_OnOpenshift(t *testing.T) { - b := NewFakeClientBuilder().OnOpenshift() - - // client.Client - assert.Len(t, b.scheme.KnownTypes(routev1.GroupVersion), 10) - assert.Contains(t, b.scheme.KnownTypes(routev1.GroupVersion), strings.Split(reflect.TypeOf(&routev1.Route{}).String(), ".")[1]) - assert.Contains(t, b.scheme.KnownTypes(routev1.GroupVersion), strings.Split(reflect.TypeOf(&routev1.RouteList{}).String(), ".")[1]) - - // discovery.DiscoveryInterface - assert.True(t, resourceListsContainsGroupVersion(b.resources, routev1.GroupVersion.String())) - assert.True(t, resourceListsContainsGroupVersion(b.resources, openshiftGroupVersion)) -} - -func TestFakeClientBuilder_WithIngress(t *testing.T) { - b := NewFakeClientBuilder().WithIngress() - - // client.Client - assert.Len(t, b.scheme.KnownTypes(networkingv1.SchemeGroupVersion), 14) - assert.Contains(t, b.scheme.KnownTypes(networkingv1.SchemeGroupVersion), strings.Split(reflect.TypeOf(&networkingv1.Ingress{}).String(), ".")[1]) - assert.Contains(t, b.scheme.KnownTypes(networkingv1.SchemeGroupVersion), strings.Split(reflect.TypeOf(&networkingv1.IngressList{}).String(), ".")[1]) - - // discovery.DiscoveryInterface - assert.True(t, resourceListsContainsGroupVersion(b.resources, networkingv1.SchemeGroupVersion.String())) -} - -func TestFakeClientBuilder_WithLegacyIngress(t *testing.T) { - b := NewFakeClientBuilder().WithLegacyIngress() - - // client.Client - assert.Len(t, b.scheme.KnownTypes(networkingv1beta1.SchemeGroupVersion), 12) - assert.Contains(t, b.scheme.KnownTypes(networkingv1beta1.SchemeGroupVersion), strings.Split(reflect.TypeOf(&networkingv1beta1.Ingress{}).String(), ".")[1]) - assert.Contains(t, b.scheme.KnownTypes(networkingv1beta1.SchemeGroupVersion), strings.Split(reflect.TypeOf(&networkingv1beta1.IngressList{}).String(), ".")[1]) - - // discovery.DiscoveryInterface - assert.True(t, resourceListsContainsGroupVersion(b.resources, networkingv1beta1.SchemeGroupVersion.String())) -} - -func TestFakeClientBuilder_Build(t *testing.T) { - nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "nexus"}} - route := &routev1.Route{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "route"}} - ingress := &networkingv1beta1.Ingress{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "ingress"}} - - b := NewFakeClientBuilder(nexus) - c := b.Build() - discovery.SetClient(c) - assert.NotNil(t, c.disc) - assert.NotNil(t, c.client) - assert.NoError(t, c.client.Get(ctx.TODO(), client.ObjectKey{ - Namespace: nexus.Namespace, - Name: nexus.Name, - }, nexus)) - ocp, _ := discovery.IsOpenShift() - assert.False(t, ocp) - withRoute, _ := discovery.IsRouteAvailable() - assert.False(t, withRoute) - withIngress, _ := discovery.IsIngressAvailable() - assert.False(t, withIngress) - withLegacyIngress, _ := discovery.IsLegacyIngressAvailable() - assert.False(t, withLegacyIngress) - - // on Openshift - b = NewFakeClientBuilder(nexus, route).OnOpenshift() - c = b.Build() - discovery.SetClient(c) - assert.NoError(t, c.client.Get(ctx.TODO(), client.ObjectKey{ - Namespace: route.Namespace, - Name: route.Name, - }, route)) - ocp, _ = discovery.IsOpenShift() - assert.True(t, ocp) - withRoute, _ = discovery.IsRouteAvailable() - assert.True(t, withRoute) - withIngress, _ = discovery.IsIngressAvailable() - assert.False(t, withIngress) - withLegacyIngress, _ = discovery.IsLegacyIngressAvailable() - assert.False(t, withLegacyIngress) - - // with Ingress - b = NewFakeClientBuilder(nexus, ingress).WithIngress() - c = b.Build() - discovery.SetClient(c) - assert.NoError(t, c.client.Get(ctx.TODO(), client.ObjectKey{ - Namespace: ingress.Namespace, - Name: ingress.Name, - }, ingress)) - ocp, _ = discovery.IsOpenShift() - assert.False(t, ocp) - withRoute, _ = discovery.IsRouteAvailable() - assert.False(t, withRoute) - withIngress, _ = discovery.IsIngressAvailable() - assert.True(t, withIngress) - withLegacyIngress, _ = discovery.IsLegacyIngressAvailable() - assert.False(t, withLegacyIngress) - - // with v1beta1 Ingress - b = NewFakeClientBuilder(nexus, ingress).WithLegacyIngress() - c = b.Build() - discovery.SetClient(c) - assert.NoError(t, c.client.Get(ctx.TODO(), client.ObjectKey{ - Namespace: ingress.Namespace, - Name: ingress.Name, - }, ingress)) - ocp, _ = discovery.IsOpenShift() - assert.False(t, ocp) - withRoute, _ = discovery.IsRouteAvailable() - assert.False(t, withRoute) - withIngress, _ = discovery.IsIngressAvailable() - assert.False(t, withIngress) - withLegacyIngress, _ = discovery.IsLegacyIngressAvailable() - assert.True(t, withLegacyIngress) -} - -func resourceListsContainsGroupVersion(lists []*metav1.APIResourceList, gv string) bool { - for _, list := range lists { - if list.GroupVersion == gv { - return true - } - } - return false -} - -func TestFakeClient_SetMockError(t *testing.T) { - c := NewFakeClientBuilder().Build() - assert.Nil(t, c.mockErr) - assert.False(t, c.shouldClearError) - c.SetMockError(fmt.Errorf(testErrorMsg)) - assert.Equal(t, c.mockErr.Error(), testErrorMsg) - assert.False(t, c.shouldClearError) -} - -func TestFakeClient_SetMockErrorForOneRequest(t *testing.T) { - c := NewFakeClientBuilder().Build() - assert.Nil(t, c.mockErr) - assert.False(t, c.shouldClearError) - c.SetMockErrorForOneRequest(fmt.Errorf(testErrorMsg)) - assert.Equal(t, c.mockErr.Error(), testErrorMsg) - assert.True(t, c.shouldClearError) -} - -func TestFakeClient_ClearMockError(t *testing.T) { - c := NewFakeClientBuilder().Build() - assert.Nil(t, c.mockErr) - assert.False(t, c.shouldClearError) - c.SetMockErrorForOneRequest(fmt.Errorf(testErrorMsg)) - assert.Equal(t, c.mockErr.Error(), testErrorMsg) - assert.True(t, c.shouldClearError) - c.ClearMockError() - assert.Nil(t, c.mockErr) - assert.False(t, c.shouldClearError) -} - -func TestFakeClient_RESTClient(t *testing.T) { - c := NewFakeClientBuilder().Build() - assert.Equal(t, c.disc.RESTClient(), c.RESTClient()) -} - -func TestFakeClient_ServerGroups(t *testing.T) { - c := NewFakeClientBuilder().Build() - mockErr := fmt.Errorf(testErrorMsg) - c.SetMockErrorForOneRequest(mockErr) - _, err := c.ServerGroups() - assert.Equal(t, mockErr, err) - - want, wantErr := c.disc.ServerGroups() - got, gotErr := c.ServerGroups() - assert.Equal(t, want, got) - assert.Equal(t, wantErr, gotErr) - assert.NotEqual(t, gotErr, mockErr) -} - -func TestFakeClient_ServerResourcesForGroupVersion(t *testing.T) { - c := NewFakeClientBuilder().Build() - mockErr := fmt.Errorf(testErrorMsg) - c.SetMockErrorForOneRequest(mockErr) - _, err := c.ServerResourcesForGroupVersion("") - assert.Equal(t, mockErr, err) - - want, wantErr := c.disc.ServerResourcesForGroupVersion("") - got, gotErr := c.ServerResourcesForGroupVersion("") - assert.Equal(t, want, got) - assert.Equal(t, wantErr, gotErr) - assert.NotEqual(t, gotErr, mockErr) -} - -// Deprecated: use ServerGroupsAndResources instead. -func TestFakeClient_ServerResources(t *testing.T) { - c := NewFakeClientBuilder().Build() - mockErr := fmt.Errorf(testErrorMsg) - c.SetMockErrorForOneRequest(mockErr) - _, err := c.ServerResources() - assert.Equal(t, mockErr, err) - - want, wantErr := c.disc.ServerResources() - got, gotErr := c.ServerResources() - assert.Equal(t, want, got) - assert.Equal(t, wantErr, gotErr) - assert.NotEqual(t, gotErr, mockErr) -} - -func TestFakeClient_ServerGroupsAndResources(t *testing.T) { - c := NewFakeClientBuilder().Build() - mockErr := fmt.Errorf(testErrorMsg) - c.SetMockErrorForOneRequest(mockErr) - _, _, err := c.ServerGroupsAndResources() - assert.Equal(t, mockErr, err) - - want1, want2, wantErr := c.disc.ServerGroupsAndResources() - got1, got2, gotErr := c.ServerGroupsAndResources() - assert.Equal(t, want1, got1) - assert.Equal(t, want2, got2) - assert.Equal(t, wantErr, gotErr) - assert.NotEqual(t, gotErr, mockErr) -} - -func TestFakeClient_ServerPreferredResources(t *testing.T) { - c := NewFakeClientBuilder().Build() - mockErr := fmt.Errorf(testErrorMsg) - c.SetMockErrorForOneRequest(mockErr) - _, err := c.ServerPreferredResources() - assert.Equal(t, mockErr, err) - - want, wantErr := c.disc.ServerPreferredResources() - got, gotErr := c.ServerPreferredResources() - assert.Equal(t, want, got) - assert.Equal(t, wantErr, gotErr) - assert.NotEqual(t, gotErr, mockErr) -} - -func TestFakeClient_ServerPreferredNamespacedResources(t *testing.T) { - c := NewFakeClientBuilder().Build() - mockErr := fmt.Errorf(testErrorMsg) - c.SetMockErrorForOneRequest(mockErr) - _, err := c.ServerPreferredNamespacedResources() - assert.Equal(t, mockErr, err) - - want, wantErr := c.disc.ServerPreferredNamespacedResources() - got, gotErr := c.ServerPreferredNamespacedResources() - assert.Equal(t, want, got) - assert.Equal(t, wantErr, gotErr) - assert.NotEqual(t, gotErr, mockErr) -} - -func TestFakeClient_ServerVersion(t *testing.T) { - c := NewFakeClientBuilder().Build() - mockErr := fmt.Errorf(testErrorMsg) - c.SetMockErrorForOneRequest(mockErr) - _, err := c.ServerVersion() - assert.Equal(t, mockErr, err) - - want, wantErr := c.disc.ServerVersion() - got, gotErr := c.ServerVersion() - assert.Equal(t, want, got) - assert.Equal(t, wantErr, gotErr) - assert.NotEqual(t, gotErr, mockErr) -} - -func TestFakeClient_OpenAPISchema(t *testing.T) { - c := NewFakeClientBuilder().Build() - mockErr := fmt.Errorf(testErrorMsg) - c.SetMockErrorForOneRequest(mockErr) - _, err := c.OpenAPISchema() - assert.Equal(t, mockErr, err) - - want, wantErr := c.disc.OpenAPISchema() - got, gotErr := c.OpenAPISchema() - assert.Equal(t, want, got) - assert.Equal(t, wantErr, gotErr) - assert.NotEqual(t, gotErr, mockErr) +func TestNewFakeClient(t *testing.T) { + dep := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "dep", Namespace: t.Name()}} + key := framework.Key(dep) + c := NewFakeClient(dep) + assert.False(t, IsInterfaceValueNil(c.client)) // make sure there actually is a client + assert.NoError(t, c.Get(ctx.Background(), key, dep)) } func TestFakeClient_Get(t *testing.T) { - c := NewFakeClientBuilder().Build() + c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) err := c.Get(ctx.TODO(), client.ObjectKey{}, &v1alpha1.Nexus{}) @@ -335,7 +52,7 @@ func TestFakeClient_Get(t *testing.T) { } func TestFakeClient_List(t *testing.T) { - c := NewFakeClientBuilder().Build() + c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) err := c.List(ctx.TODO(), &v1alpha1.NexusList{}) @@ -348,7 +65,7 @@ func TestFakeClient_List(t *testing.T) { } func TestFakeClient_Create(t *testing.T) { - c := NewFakeClientBuilder().Build() + c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) err := c.Create(ctx.TODO(), &v1alpha1.Nexus{}) @@ -361,7 +78,7 @@ func TestFakeClient_Create(t *testing.T) { } func TestFakeClient_Delete(t *testing.T) { - c := NewFakeClientBuilder().Build() + c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) err := c.Delete(ctx.TODO(), &v1alpha1.Nexus{}) @@ -374,7 +91,7 @@ func TestFakeClient_Delete(t *testing.T) { } func TestFakeClient_Update(t *testing.T) { - c := NewFakeClientBuilder().Build() + c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) err := c.Update(ctx.TODO(), &v1alpha1.Nexus{}) @@ -387,7 +104,7 @@ func TestFakeClient_Update(t *testing.T) { } func TestFakeClient_Patch(t *testing.T) { - c := NewFakeClientBuilder().Build() + c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) err := c.Patch(ctx.TODO(), &v1alpha1.Nexus{}, client.MergeFrom(&v1alpha1.Nexus{})) @@ -400,7 +117,7 @@ func TestFakeClient_Patch(t *testing.T) { } func TestFakeClient_DeleteAllOf(t *testing.T) { - c := NewFakeClientBuilder().Build() + c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) err := c.DeleteAllOf(ctx.TODO(), &v1alpha1.Nexus{}) @@ -413,11 +130,6 @@ func TestFakeClient_DeleteAllOf(t *testing.T) { } func TestFakeClient_Status(t *testing.T) { - c := NewFakeClientBuilder().Build() + c := NewFakeClient() assert.Equal(t, c.client.Status(), c.Status()) } - -func TestFakeClient_Scheme(t *testing.T) { - c := NewFakeClientBuilder().Build() - assert.Equal(t, c.scheme, c.Scheme()) -} diff --git a/pkg/test/utils.go b/pkg/test/utils.go index f845fcc9..4059bbaf 100644 --- a/pkg/test/utils.go +++ b/pkg/test/utils.go @@ -24,6 +24,7 @@ import ( "github.com/RHsyseng/operator-utils/pkg/resource" ) +// ContainsType returns true if the give resource slice contains an element of type t func ContainsType(resources []resource.KubernetesResource, t reflect.Type) bool { for _, res := range resources { if reflect.TypeOf(res) == t { @@ -33,6 +34,7 @@ func ContainsType(resources []resource.KubernetesResource, t reflect.Type) bool return false } +// EventExists returns true if an event with the given reason exists func EventExists(c client.Client, reason string) bool { eventList := &v1.EventList{} _ = c.List(context.Background(), eventList) @@ -43,3 +45,9 @@ func EventExists(c client.Client, reason string) bool { } return false } + +// IsInterfaceValueNil returns true if the value stored by the interface is nil +// See https://medium.com/@glucn/golang-an-interface-holding-a-nil-value-is-not-nil-bb151f472cc7 +func IsInterfaceValueNil(i interface{}) bool { + return i == nil || (reflect.ValueOf(i).Kind() == reflect.Ptr && reflect.ValueOf(i).IsNil()) +} diff --git a/pkg/test/utils_test.go b/pkg/test/utils_test.go index a4dede3a..e11900fd 100644 --- a/pkg/test/utils_test.go +++ b/pkg/test/utils_test.go @@ -28,3 +28,30 @@ func TestContainsType(t *testing.T) { assert.True(t, ContainsType(resources, reflect.TypeOf(&corev1.ServiceAccount{}))) assert.False(t, ContainsType(resources, reflect.TypeOf(&corev1.Service{}))) } + +func TestEventExists(t *testing.T) { + testReason := "reason" + testEvent := &corev1.Event{Reason: testReason} + c := NewFakeClient(testEvent) + + assert.False(t, EventExists(c, "some other reason")) + assert.True(t, EventExists(c, testReason)) +} + +type foo interface { + bar() +} +type concrete struct{} + +func (*concrete) bar() {} +func TestIsInterfaceValueNil(t *testing.T) { + var f foo + assert.True(t, IsInterfaceValueNil(f)) + + var c *concrete + f = c + assert.True(t, IsInterfaceValueNil(f)) + + f = &concrete{} + assert.False(t, IsInterfaceValueNil(f)) +} diff --git a/pkg/util/error.go b/pkg/util/error.go deleted file mode 100644 index c891651b..00000000 --- a/pkg/util/error.go +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2020 Nexus Operator and/or its 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 util - -// Must panics on non-nil errors. -func Must(err error) { - if err != nil { - panic(err) - } -} From 952b87b89833554af08b2c3de7154175d19c3b6c Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Tue, 22 Dec 2020 16:31:08 -0300 Subject: [PATCH 03/10] Fix tests --- Makefile | 8 +- api/v1alpha1/defaults.go | 7 - api/v1alpha1/mutation.go | 18 +- api/v1alpha1/mutation_test.go | 338 +++++++++++++++ api/v1alpha1/validation.go | 11 +- api/v1alpha1/validation_test.go | 400 ++---------------- api/v1alpha1/zz_generated.deepcopy.go | 2 +- config/webhook/manifests.yaml | 52 +++ .../nexus/resource/deployment/manager.go | 3 +- .../nexus/resource/deployment/manager_test.go | 11 +- .../nexus/resource/networking/manager.go | 3 +- .../nexus/resource/networking/manager_test.go | 41 +- .../nexus/resource/persistence/manager.go | 3 +- .../resource/persistence/manager_test.go | 11 +- .../nexus/resource/security/manager.go | 3 +- .../nexus/resource/security/manager_test.go | 22 +- controllers/nexus/server/nexus_test.go | 16 +- controllers/nexus/server/users.go | 5 +- controllers/nexus/update/events_test.go | 26 +- controllers/nexus/update/monitor_test.go | 3 +- controllers/nexus_controller_test.go | 3 + hack/go-fmt.sh | 4 +- pkg/{test/client.go => client/fake.go} | 10 +- .../client_test.go => client/fake_test.go} | 48 +-- pkg/{framework => client}/fetcher.go | 6 +- pkg/{framework => client}/fetcher_test.go | 31 +- pkg/cluster/discovery/fake.go | 14 + pkg/cluster/discovery/fake_test.go | 14 + pkg/cluster/discovery/kubernetes.go | 14 + pkg/cluster/discovery/kubernetes_test.go | 65 +++ pkg/cluster/kubernetes/events_test.go | 24 +- pkg/test/utils_test.go | 4 +- 32 files changed, 702 insertions(+), 518 deletions(-) create mode 100644 config/webhook/manifests.yaml rename pkg/{test/client.go => client/fake.go} (94%) rename pkg/{test/client_test.go => client/fake_test.go} (66%) rename pkg/{framework => client}/fetcher.go (95%) rename pkg/{framework => client}/fetcher_test.go (71%) diff --git a/Makefile b/Makefile index f35682f2..9ba96a2f 100644 --- a/Makefile +++ b/Makefile @@ -36,13 +36,17 @@ all: manager ENVTEST_ASSETS_DIR=$(shell pwd)/testbin # Needed to support k8s.io/api/networking/v1 Ingress K8S_VERSION=1.19.0 -test: generate-installer fmt vet bundle - mkdir -p ${ENVTEST_ASSETS_DIR} +test: generate-installer fmt vet bundle test-only + +# just test without generating anything, use wisely +test-only: + mkdir -p ${ENVTEST_ASSETS_DIR} test -f ${ENVTEST_ASSETS_DIR}/setup-envtest.sh || curl -sSLo ${ENVTEST_ASSETS_DIR}/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/master/hack/setup-envtest.sh sed -i "s,#\!.*,#\!\/bin\/bash,g" ${ENVTEST_ASSETS_DIR}/setup-envtest.sh sed -i "/pipefail/d" ${ENVTEST_ASSETS_DIR}/setup-envtest.sh source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; ENVTEST_K8S_VERSION=$(K8S_VERSION) fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out + generate-installer: generate manifests kustomize cd config/manager && $(KUSTOMIZE) edit set image controller=$(OPERATOR_IMG) $(KUSTOMIZE) build config/default > nexus-operator.yaml diff --git a/api/v1alpha1/defaults.go b/api/v1alpha1/defaults.go index 4b09db28..ae5878ca 100644 --- a/api/v1alpha1/defaults.go +++ b/api/v1alpha1/defaults.go @@ -55,12 +55,6 @@ var ( FailureThreshold: probeDefaultFailureThreshold, } - DefaultPersistence = NexusPersistence{ - Persistent: false, - VolumeSize: DefaultVolumeSize, - StorageClass: "", - } - DefaultNetworking = NexusNetworking{ Expose: false, TLS: DefaultTLS, @@ -84,7 +78,6 @@ var ( ImagePullPolicy: "", AutomaticUpdate: DefaultUpdate, Resources: DefaultResources, - Persistence: DefaultPersistence, UseRedHatImage: false, GenerateRandomAdminPassword: false, Networking: DefaultNetworking, diff --git a/api/v1alpha1/mutation.go b/api/v1alpha1/mutation.go index 0b4684bf..ba4170d2 100644 --- a/api/v1alpha1/mutation.go +++ b/api/v1alpha1/mutation.go @@ -32,22 +32,16 @@ func newMutator(nexus *Nexus) *mutator { log.Error(err, "Unable to determine if Routes are available. Will assume they're not.") } - ingressAvailable, err := discovery.IsIngressAvailable() + ingressAvailable, err := discovery.IsAnyIngressAvailable() if err != nil { - log.Error(err, "Unable to determine if v1 Ingresses are available. Will assume they're not.") - } - - legacyIngressAvailable, err := discovery.IsLegacyIngressAvailable() - if err != nil { - log.Error(err, "Unable to determine if v1beta1 Ingresses are available. Will assume they're not.") + log.Error(err, "Unable to determine if Ingresses are available. Will assume they're not.") } return &mutator{ - nexus: nexus, - log: log, - routeAvailable: routeAvailable, - // TODO: create a IsAnyIngressAvailable function in discovery, we don't care about which one in admission - ingressAvailable: ingressAvailable || legacyIngressAvailable, + nexus: nexus, + log: log, + routeAvailable: routeAvailable, + ingressAvailable: ingressAvailable, } } diff --git a/api/v1alpha1/mutation_test.go b/api/v1alpha1/mutation_test.go index 1c267880..7907bda7 100644 --- a/api/v1alpha1/mutation_test.go +++ b/api/v1alpha1/mutation_test.go @@ -1 +1,339 @@ package v1alpha1 + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/m88i/nexus-operator/pkg/cluster/discovery" + "github.com/m88i/nexus-operator/pkg/framework" +) + +func TestNewMutator(t *testing.T) { + tests := []struct { + name string + client *discovery.FakeDisc + want *validator + }{ + { + "On OCP", + discovery.NewFakeDiscBuilder().OnOpenshift().Build(), + &validator{ + routeAvailable: true, + ingressAvailable: false, + }, + }, + { + "On K8s, no ingress", + discovery.NewFakeDiscBuilder().Build(), + &validator{ + routeAvailable: false, + ingressAvailable: false, + }, + }, + { + "On K8s with v1beta1 ingress", + discovery.NewFakeDiscBuilder().WithLegacyIngress().Build(), + &validator{ + routeAvailable: false, + ingressAvailable: true, + }, + }, + { + "On K8s with v1 ingress", + discovery.NewFakeDiscBuilder().WithIngress().Build(), + &validator{ + routeAvailable: false, + ingressAvailable: true, + }, + }, + } + + // the nexus itself isn't important, but we need it to avoid nil dereference + n := &Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus", Namespace: t.Name()}} + for _, tt := range tests { + discovery.SetClient(tt.client) + m := newMutator(n) + assert.Equal(t, tt.want.ingressAvailable, m.ingressAvailable) + assert.Equal(t, tt.want.routeAvailable, m.routeAvailable) + } + + // now let's test out error + client := discovery.NewFakeDiscBuilder().WithIngress().Build() + errString := "test error" + client.SetMockError(fmt.Errorf(errString)) + discovery.SetClient(client) + got := newMutator(n) + // all calls to discovery failed, so all should be false + assert.False(t, got.routeAvailable) + assert.False(t, got.ingressAvailable) +} + +func TestMutator_mutate_AutomaticUpdate(t *testing.T) { + discovery.SetClient(discovery.NewFakeDiscBuilder().Build()) + nexus := &Nexus{Spec: NexusSpec{AutomaticUpdate: NexusAutomaticUpdate{}}} + nexus.Spec.Image = NexusCommunityImage + newMutator(nexus).mutate() + latestMinor, err := framework.GetLatestMinor() + if err != nil { + // If we couldn't fetch the tags updates should be disabled + assert.True(t, nexus.Spec.AutomaticUpdate.Disabled) + } else { + assert.Equal(t, latestMinor, *nexus.Spec.AutomaticUpdate.MinorVersion) + } + + // Now an invalid image + nexus = &Nexus{Spec: NexusSpec{AutomaticUpdate: NexusAutomaticUpdate{}}} + nexus.Spec.Image = "some-image" + newMutator(nexus).mutate() + assert.True(t, nexus.Spec.AutomaticUpdate.Disabled) + + // Informed a minor which does not exist + nexus = &Nexus{Spec: NexusSpec{AutomaticUpdate: NexusAutomaticUpdate{}}} + nexus.Spec.Image = NexusCommunityImage + bogusMinor := -1 + nexus.Spec.AutomaticUpdate.MinorVersion = &bogusMinor + newMutator(nexus).mutate() + latestMinor, err = framework.GetLatestMinor() + if err != nil { + // If we couldn't fetch the tags updates should be disabled + assert.True(t, nexus.Spec.AutomaticUpdate.Disabled) + } else { + assert.Equal(t, latestMinor, *nexus.Spec.AutomaticUpdate.MinorVersion) + } +} + +func TestMutator_mutate_Networking(t *testing.T) { + tests := []struct { + name string + disc *discovery.FakeDisc + input *Nexus + want *Nexus + }{ + { + "'spec.networking.exposeAs' left blank on OCP", + discovery.NewFakeDiscBuilder().OnOpenshift().Build(), + func() *Nexus { + n := AllDefaultsCommunityNexus.DeepCopy() + n.Spec.Networking.Expose = true + return n + }(), + func() *Nexus { + n := AllDefaultsCommunityNexus.DeepCopy() + n.Spec.Networking.Expose = true + n.Spec.Networking.ExposeAs = RouteExposeType + return n + }(), + }, + { + "'spec.networking.exposeAs' left blank on K8s with ingress", + discovery.NewFakeDiscBuilder().WithIngress().Build(), + func() *Nexus { + n := AllDefaultsCommunityNexus.DeepCopy() + n.Spec.Networking.Expose = true + return n + }(), + func() *Nexus { + n := AllDefaultsCommunityNexus.DeepCopy() + n.Spec.Networking.Expose = true + n.Spec.Networking.ExposeAs = IngressExposeType + return n + }(), + }, + { + "'spec.networking.exposeAs' left blank on K8s, but Ingress unavailable", + discovery.NewFakeDiscBuilder().Build(), + func() *Nexus { + n := AllDefaultsCommunityNexus.DeepCopy() + n.Spec.Networking.Expose = true + return n + }(), + func() *Nexus { + n := AllDefaultsCommunityNexus.DeepCopy() + n.Spec.Networking.Expose = true + n.Spec.Networking.ExposeAs = NodePortExposeType + return n + }(), + }, + } + + for _, tt := range tests { + discovery.SetClient(tt.disc) + newMutator(tt.input).mutate() + assert.Equal(t, tt.want, tt.input) + } +} + +func TestMutator_mutate_Persistence(t *testing.T) { + tests := []struct { + name string + input *Nexus + want *Nexus + }{ + { + "'spec.persistence.volumeSize' left blank", + func() *Nexus { + n := AllDefaultsCommunityNexus.DeepCopy() + n.Spec.Persistence.Persistent = true + return n + }(), + func() *Nexus { + n := AllDefaultsCommunityNexus.DeepCopy() + n.Spec.Persistence.Persistent = true + n.Spec.Persistence.VolumeSize = DefaultVolumeSize + return n + }(), + }, + } + + discovery.SetClient(discovery.NewFakeDiscBuilder().Build()) + for _, tt := range tests { + newMutator(tt.input).mutate() + assert.Equal(t, tt.want, tt.input) + } +} + +func TestMutator_mutate_Security(t *testing.T) { + tests := []struct { + name string + input *Nexus + want *Nexus + }{ + { + "'spec.serviceAccountName' left blank", + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.ServiceAccountName = "" + return nexus + }(), + &AllDefaultsCommunityNexus, + }, + } + + discovery.SetClient(discovery.NewFakeDiscBuilder().Build()) + for _, tt := range tests { + newMutator(tt.input).mutate() + assert.Equal(t, tt.want, tt.input) + } +} + +func TestMutator_mutate_Deployment(t *testing.T) { + minimumDefaultProbe := &NexusProbe{ + InitialDelaySeconds: 0, + TimeoutSeconds: 1, + PeriodSeconds: 1, + SuccessThreshold: 1, + FailureThreshold: 1, + } + + tests := []struct { + name string + input *Nexus + want *Nexus + }{ + { + "'spec.resources' left blank", + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.Resources = v1.ResourceRequirements{} + return nexus + }(), + &AllDefaultsCommunityNexus, + }, + { + "'spec.useRedHatImage' set to true and 'spec.image' not left blank", + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.UseRedHatImage = true + nexus.Spec.Image = "some-image" + return nexus + }(), + func() *Nexus { + n := AllDefaultsCommunityNexus.DeepCopy() + n.Spec.UseRedHatImage = true + n.Spec.Image = NexusCertifiedImage + return n + }(), + }, + { + "'spec.useRedHatImage' set to false and 'spec.image' left blank", + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.Image = "" + return nexus + }(), + &AllDefaultsCommunityNexus, + }, + { + "'spec.livenessProbe.successThreshold' not equal to 1", + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.LivenessProbe.SuccessThreshold = 2 + return nexus + }(), + &AllDefaultsCommunityNexus, + }, + { + "'spec.livenessProbe.*' and 'spec.readinessProbe.*' don't meet minimum values", + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.LivenessProbe = &NexusProbe{ + InitialDelaySeconds: -1, + TimeoutSeconds: -1, + PeriodSeconds: -1, + SuccessThreshold: -1, + FailureThreshold: -1, + } + nexus.Spec.ReadinessProbe = nexus.Spec.LivenessProbe.DeepCopy() + return nexus + }(), + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.LivenessProbe = minimumDefaultProbe.DeepCopy() + nexus.Spec.ReadinessProbe = minimumDefaultProbe.DeepCopy() + return nexus + }(), + }, + { + "Unset 'spec.livenessProbe' and 'spec.readinessProbe'", + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.LivenessProbe = nil + nexus.Spec.ReadinessProbe = nil + return nexus + }(), + AllDefaultsCommunityNexus.DeepCopy(), + }, + { + "Invalid 'spec.imagePullPolicy'", + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.ImagePullPolicy = "invalid" + return nexus + }(), + AllDefaultsCommunityNexus.DeepCopy(), + }, + { + "Invalid 'spec.replicas'", + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.Replicas = 3 + return nexus + }(), + func() *Nexus { + nexus := AllDefaultsCommunityNexus.DeepCopy() + nexus.Spec.Replicas = 1 + return nexus + }(), + }, + } + + discovery.SetClient(discovery.NewFakeDiscBuilder().Build()) + for _, tt := range tests { + newMutator(tt.input).mutate() + assert.Equal(t, tt.want, tt.input) + } +} diff --git a/api/v1alpha1/validation.go b/api/v1alpha1/validation.go index 9f82ba9d..e7b5acb5 100644 --- a/api/v1alpha1/validation.go +++ b/api/v1alpha1/validation.go @@ -38,21 +38,16 @@ func newValidator(nexus *Nexus) *validator { log.Error(err, "Unable to determine if Routes are available. Will assume they're not.") } - ingressAvailable, err := discovery.IsIngressAvailable() + ingressAvailable, err := discovery.IsAnyIngressAvailable() if err != nil { - log.Error(err, "Unable to determine if v1 Ingresses are available. Will assume they're not.") - } - - legacyIngressAvailable, err := discovery.IsLegacyIngressAvailable() - if err != nil { - log.Error(err, "Unable to determine if v1beta1 Ingresses are available. Will assume they're not.") + log.Error(err, "Unable to determine if Ingresses are available. Will assume they're not.") } return &validator{ nexus: nexus, log: log, routeAvailable: routeAvailable, - ingressAvailable: ingressAvailable || legacyIngressAvailable, + ingressAvailable: ingressAvailable, } } diff --git a/api/v1alpha1/validation_test.go b/api/v1alpha1/validation_test.go index f546ddb7..3a3f8b66 100644 --- a/api/v1alpha1/validation_test.go +++ b/api/v1alpha1/validation_test.go @@ -16,29 +16,24 @@ package v1alpha1 import ( "fmt" - "reflect" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" "github.com/m88i/nexus-operator/pkg/cluster/discovery" - "github.com/m88i/nexus-operator/pkg/framework" - "github.com/m88i/nexus-operator/pkg/logger" ) -// TODO: fix the tests -// TODO: move mutation tests to mutation_test.go - func TestNewValidator(t *testing.T) { tests := []struct { name string - client *test.FakeClient + client *discovery.FakeDisc want *validator }{ { "On OCP", - test.NewFakeClientBuilder().OnOpenshift().Build(), + discovery.NewFakeDiscBuilder().OnOpenshift().Build(), &validator{ routeAvailable: true, ingressAvailable: false, @@ -46,7 +41,7 @@ func TestNewValidator(t *testing.T) { }, { "On K8s, no ingress", - test.NewFakeClientBuilder().Build(), + discovery.NewFakeDiscBuilder().Build(), &validator{ routeAvailable: false, ingressAvailable: false, @@ -54,7 +49,7 @@ func TestNewValidator(t *testing.T) { }, { "On K8s with v1beta1 ingress", - test.NewFakeClientBuilder().WithLegacyIngress().Build(), + discovery.NewFakeDiscBuilder().WithLegacyIngress().Build(), &validator{ routeAvailable: false, ingressAvailable: true, @@ -62,7 +57,7 @@ func TestNewValidator(t *testing.T) { }, { "On K8s with v1 ingress", - test.NewFakeClientBuilder().WithIngress().Build(), + discovery.NewFakeDiscBuilder().WithIngress().Build(), &validator{ routeAvailable: false, ingressAvailable: true, @@ -70,440 +65,119 @@ func TestNewValidator(t *testing.T) { }, } + // the nexus itself isn't important, but we need it to avoid nil dereference + n := &Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus", Namespace: t.Name()}} for _, tt := range tests { discovery.SetClient(tt.client) - // the nexus CR is not important here - assert.Equal(t, tt.want, newValidator(nil)) + v := newValidator(n) + assert.Equal(t, tt.want.ingressAvailable, v.ingressAvailable) + assert.Equal(t, tt.want.routeAvailable, v.routeAvailable) } // now let's test out error - client := test.NewFakeClientBuilder().WithIngress().Build() + client := discovery.NewFakeDiscBuilder().WithIngress().Build() errString := "test error" client.SetMockError(fmt.Errorf(errString)) discovery.SetClient(client) - got := newValidator(nil) + got := newValidator(n) // all calls to discovery failed, so all should be false assert.False(t, got.routeAvailable) assert.False(t, got.ingressAvailable) } -func TestValidator_SetDefaultsAndValidate_Deployment(t *testing.T) { - minimumDefaultProbe := &NexusProbe{ - InitialDelaySeconds: 0, - TimeoutSeconds: 1, - PeriodSeconds: 1, - SuccessThreshold: 1, - FailureThreshold: 1, - } - +func TestValidator_validate_Networking(t *testing.T) { tests := []struct { - name string - input *Nexus - want *Nexus - }{ - { - "'spec.resources' left blank", - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.Resources = corev1.ResourceRequirements{} - return nexus - }(), - &AllDefaultsCommunityNexus, - }, - { - "'spec.useRedHatImage' set to true and 'spec.image' not left blank", - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.UseRedHatImage = true - nexus.Spec.Image = "some-image" - return nexus - }(), - func() *Nexus { - n := AllDefaultsCommunityNexus.DeepCopy() - n.Spec.UseRedHatImage = true - n.Spec.Image = NexusCertifiedImage - return n - }(), - }, - { - "'spec.useRedHatImage' set to false and 'spec.image' left blank", - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.Image = "" - return nexus - }(), - &AllDefaultsCommunityNexus, - }, - { - "'spec.livenessProbe.successThreshold' not equal to 1", - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.LivenessProbe.SuccessThreshold = 2 - return nexus - }(), - &AllDefaultsCommunityNexus, - }, - { - "'spec.livenessProbe.*' and 'spec.readinessProbe.*' don't meet minimum values", - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.LivenessProbe = &NexusProbe{ - InitialDelaySeconds: -1, - TimeoutSeconds: -1, - PeriodSeconds: -1, - SuccessThreshold: -1, - FailureThreshold: -1, - } - nexus.Spec.ReadinessProbe = nexus.Spec.LivenessProbe.DeepCopy() - return nexus - }(), - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.LivenessProbe = minimumDefaultProbe.DeepCopy() - nexus.Spec.ReadinessProbe = minimumDefaultProbe.DeepCopy() - return nexus - }(), - }, - { - "Unset 'spec.livenessProbe' and 'spec.readinessProbe'", - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.LivenessProbe = nil - nexus.Spec.ReadinessProbe = nil - return nexus - }(), - AllDefaultsCommunityNexus.DeepCopy(), - }, - { - "Invalid 'spec.imagePullPolicy'", - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.ImagePullPolicy = "invalid" - return nexus - }(), - AllDefaultsCommunityNexus.DeepCopy(), - }, - { - "Invalid 'spec.replicas'", - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.Replicas = 3 - return nexus - }(), - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.Replicas = 1 - return nexus - }(), - }, - } - - for _, tt := range tests { - v := &validator{} - got, err := v.SetDefaultsAndValidate(tt.input) - assert.Nil(t, err) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("%s\nWant: %+v\nGot: %+v", tt.name, tt.want, got) - } - } -} - -func TestValidator_setUpdateDefaults(t *testing.T) { - client := test.NewFakeClientBuilder().Build() - nexus := &Nexus{Spec: NexusSpec{AutomaticUpdate: NexusAutomaticUpdate{}}} - nexus.Spec.Image = NexusCommunityImage - v, _ := newValidator(client, client.Scheme()) - v.log = logger.GetLoggerWithResource("test", nexus) - - v.setUpdateDefaults(nexus) - latestMinor, err := framework.GetLatestMinor() - if err != nil { - // If we couldn't fetch the tags updates should be disabled - assert.True(t, nexus.Spec.AutomaticUpdate.Disabled) - } else { - assert.Equal(t, latestMinor, *nexus.Spec.AutomaticUpdate.MinorVersion) - } - - // Now an invalid image - nexus = &Nexus{Spec: NexusSpec{AutomaticUpdate: NexusAutomaticUpdate{}}} - nexus.Spec.Image = "some-image" - v.setUpdateDefaults(nexus) - assert.True(t, nexus.Spec.AutomaticUpdate.Disabled) - - // Informed a minor which does not exist - nexus = &Nexus{Spec: NexusSpec{AutomaticUpdate: NexusAutomaticUpdate{}}} - nexus.Spec.Image = NexusCommunityImage - bogusMinor := -1 - nexus.Spec.AutomaticUpdate.MinorVersion = &bogusMinor - v.setUpdateDefaults(nexus) - latestMinor, err = framework.GetLatestMinor() - if err != nil { - // If we couldn't fetch the tags updates should be disabled - assert.True(t, nexus.Spec.AutomaticUpdate.Disabled) - } else { - assert.Equal(t, latestMinor, *nexus.Spec.AutomaticUpdate.MinorVersion) - } -} - -func TestValidator_setNetworkingDefaults(t *testing.T) { - tests := []struct { - name string - ocp bool - routeAvailable bool - ingressAvailable bool - input *Nexus - want *Nexus - }{ - { - "'spec.networking.exposeAs' left blank on OCP", - true, - true, - false, // unimportant - func() *Nexus { - n := AllDefaultsCommunityNexus.DeepCopy() - n.Spec.Networking.Expose = true - return n - }(), - func() *Nexus { - n := AllDefaultsCommunityNexus.DeepCopy() - n.Spec.Networking.Expose = true - n.Spec.Networking.ExposeAs = RouteExposeType - return n - }(), - }, - { - "'spec.networking.exposeAs' left blank on K8s", - false, - false, - true, - func() *Nexus { - n := AllDefaultsCommunityNexus.DeepCopy() - n.Spec.Networking.Expose = true - return n - }(), - func() *Nexus { - n := AllDefaultsCommunityNexus.DeepCopy() - n.Spec.Networking.Expose = true - n.Spec.Networking.ExposeAs = IngressExposeType - return n - }(), - }, - { - "'spec.networking.exposeAs' left blank on K8s, but Ingress unavailable", - false, - false, - false, - func() *Nexus { - n := AllDefaultsCommunityNexus.DeepCopy() - n.Spec.Networking.Expose = true - return n - }(), - func() *Nexus { - n := AllDefaultsCommunityNexus.DeepCopy() - n.Spec.Networking.Expose = true - n.Spec.Networking.ExposeAs = NodePortExposeType - return n - }(), - }, - } - - for _, tt := range tests { - v := &validator{ - routeAvailable: tt.routeAvailable, - ingressAvailable: tt.ingressAvailable, - ocp: tt.ocp, - log: logger.GetLoggerWithResource("test", tt.input), - } - got := tt.input.DeepCopy() - v.setNetworkingDefaults(got) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("%s\nWant: %v\nGot: %v", tt.name, tt.want, got) - } - } -} - -func TestValidator_validateNetworking(t *testing.T) { - tests := []struct { - name string - ocp bool - routeAvailable bool - ingressAvailable bool - input *Nexus - wantError bool + name string + disc *discovery.FakeDisc + input *Nexus + wantErr bool }{ { "'spec.networking.expose' left blank or set to false", - false, // unimportant - false, // unimportant - false, // unimportant + discovery.NewFakeDiscBuilder().Build(), + &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: false}}}, false, }, { "Valid Nexus with Ingress on K8s", - false, - false, - true, + discovery.NewFakeDiscBuilder().WithIngress().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType, Host: "example.com"}}}, false, }, { "Valid Nexus with Ingress and TLS secret on K8s", - false, - false, - true, + discovery.NewFakeDiscBuilder().WithIngress().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType, Host: "example.com", TLS: NexusNetworkingTLS{SecretName: "tt-secret"}}}}, false, }, { "Valid Nexus with Ingress on K8s, but Ingress unavailable (Kubernetes < 1.14)", - false, - false, - false, + discovery.NewFakeDiscBuilder().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType, Host: "example.com"}}}, true, }, { "Invalid Nexus with Ingress on K8s and no 'spec.networking.host'", - false, - false, - true, + discovery.NewFakeDiscBuilder().WithIngress().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType}}}, true, }, { "Invalid Nexus with Ingress on K8s and 'spec.networking.mandatory' set to 'true'", - false, - false, - true, + discovery.NewFakeDiscBuilder().WithIngress().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType, Host: "example.com", TLS: NexusNetworkingTLS{Mandatory: true}}}}, true, }, { "Invalid Nexus with Route on K8s", - false, - false, - true, + discovery.NewFakeDiscBuilder().WithIngress().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: RouteExposeType}}}, true, }, { "Valid Nexus with Route on OCP", - true, - true, - false, + discovery.NewFakeDiscBuilder().OnOpenshift().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: RouteExposeType}}}, false, }, { "Valid Nexus with Route on OCP with mandatory TLS", - true, - true, - false, + discovery.NewFakeDiscBuilder().OnOpenshift().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: RouteExposeType, TLS: NexusNetworkingTLS{Mandatory: true}}}}, false, }, { "Invalid Nexus with Route on OCP, but using secret name", - true, - true, - false, + discovery.NewFakeDiscBuilder().OnOpenshift().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: RouteExposeType, TLS: NexusNetworkingTLS{SecretName: "test-secret"}}}}, true, }, { "Invalid Nexus with Ingress on OCP", - true, - true, - false, + discovery.NewFakeDiscBuilder().OnOpenshift().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: IngressExposeType, Host: "example.com"}}}, true, }, { "Valid Nexus with Node Port", - false, // unimportant - false, // unimportant - false, // unimportant + discovery.NewFakeDiscBuilder().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: NodePortExposeType, NodePort: 8080}}}, false, }, { "Invalid Nexus with Node Port and no port informed", - false, // unimportant - false, // unimportant - false, // unimportant + discovery.NewFakeDiscBuilder().Build(), &Nexus{Spec: NexusSpec{Networking: NexusNetworking{Expose: true, ExposeAs: NodePortExposeType}}}, true, }, } for _, tt := range tests { - v := &validator{ - routeAvailable: tt.routeAvailable, - ingressAvailable: tt.ingressAvailable, - ocp: tt.ocp, - log: logger.GetLoggerWithResource("test", tt.input), - } - if err := v.validateNetworking(tt.input); (err != nil) != tt.wantError { - t.Errorf("%s\nWantError: %v\tError: %v", tt.name, tt.wantError, err) - } - } -} - -func TestValidator_SetDefaultsAndValidate_Persistence(t *testing.T) { - tests := []struct { - name string - input *Nexus - want *Nexus - }{ - { - "'spec.persistence.volumeSize' left blank", - func() *Nexus { - n := AllDefaultsCommunityNexus.DeepCopy() - n.Spec.Persistence.Persistent = true - return n - }(), - func() *Nexus { - n := AllDefaultsCommunityNexus.DeepCopy() - n.Spec.Persistence.Persistent = true - n.Spec.Persistence.VolumeSize = DefaultVolumeSize - return n - }(), - }, - } - for _, tt := range tests { - v := &validator{} - got, err := v.SetDefaultsAndValidate(tt.input) - assert.Nil(t, err) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("%s\nWant: %+v\nGot: %+v", tt.name, tt.want, got) - } - } -} - -func TestValidator_SetDefaultsAndValidate_Security(t *testing.T) { - - tests := []struct { - name string - input *Nexus - want *Nexus - }{ - { - "'spec.serviceAccountName' left blank", - func() *Nexus { - nexus := AllDefaultsCommunityNexus.DeepCopy() - nexus.Spec.ServiceAccountName = "" - return nexus - }(), - &AllDefaultsCommunityNexus, - }, - } - for _, tt := range tests { - v := &validator{} - got, err := v.SetDefaultsAndValidate(tt.input) - assert.Nil(t, err) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("%s\nWant: %+v\nGot: %+v", tt.name, tt.want, got) + discovery.SetClient(tt.disc) + err := newValidator(tt.input).validate() + if (err != nil) != tt.wantErr { + t.Errorf("%s\nnexus: %#v\nwantErr: %v\terr: %#v\n", tt.name, tt.input, tt.wantErr, err) } } } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 07b13e6f..c3f16998 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -19,7 +19,7 @@ package v1alpha1 import ( - runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml new file mode 100644 index 00000000..680da4cf --- /dev/null +++ b/config/webhook/manifests.yaml @@ -0,0 +1,52 @@ + +--- +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: MutatingWebhookConfiguration +metadata: + creationTimestamp: null + name: mutating-webhook-configuration +webhooks: +- clientConfig: + caBundle: Cg== + service: + name: webhook-service + namespace: system + path: /mutate-apps-m88i-io-m88i-io-v1alpha1-nexus + failurePolicy: Fail + name: mnexus.kb.io + rules: + - apiGroups: + - apps.m88i.io.m88i.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - nexus + +--- +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: ValidatingWebhookConfiguration +metadata: + creationTimestamp: null + name: validating-webhook-configuration +webhooks: +- clientConfig: + caBundle: Cg== + service: + name: webhook-service + namespace: system + path: /validate-apps-m88i-io-m88i-io-v1alpha1-nexus + failurePolicy: Fail + name: vnexus.kb.io + rules: + - apiGroups: + - apps.m88i.io.m88i.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - nexus diff --git a/controllers/nexus/resource/deployment/manager.go b/controllers/nexus/resource/deployment/manager.go index d96fe06d..6f164aa5 100644 --- a/controllers/nexus/resource/deployment/manager.go +++ b/controllers/nexus/resource/deployment/manager.go @@ -27,6 +27,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/m88i/nexus-operator/api/v1alpha1" + operatorclient "github.com/m88i/nexus-operator/pkg/client" "github.com/m88i/nexus-operator/pkg/framework" "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/logger" @@ -66,7 +67,7 @@ func (m *Manager) GetRequiredResources() ([]resource.KubernetesResource, error) func (m *Manager) GetDeployedResources() ([]resource.KubernetesResource, error) { var resources []resource.KubernetesResource for resType, resRef := range managedObjectsRef { - if err := framework.Fetch(m.client, framework.Key(m.nexus), resRef, resType); err == nil { + if err := operatorclient.Fetch(m.client, framework.Key(m.nexus), resRef, resType); err == nil { resources = append(resources, resRef) } else if !errors.IsNotFound(err) { return nil, fmt.Errorf("could not fetch %s (%s/%s): %v", resType, m.nexus.Namespace, m.nexus.Name, err) diff --git a/controllers/nexus/resource/deployment/manager_test.go b/controllers/nexus/resource/deployment/manager_test.go index d3770ce7..42f3c1e6 100644 --- a/controllers/nexus/resource/deployment/manager_test.go +++ b/controllers/nexus/resource/deployment/manager_test.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/m88i/nexus-operator/api/v1alpha1" + "github.com/m88i/nexus-operator/pkg/client" "github.com/m88i/nexus-operator/pkg/logger" "github.com/m88i/nexus-operator/pkg/test" ) @@ -49,12 +50,12 @@ func TestNewManager(t *testing.T) { // default-setting logic is tested elsewhere // so here we just check if the resulting manager took in the arguments correctly nexus := allDefaultsCommunityNexus - client := test.NewFakeClientBuilder().Build() + c := client.NewFakeClient() want := &Manager{ nexus: nexus, - client: client, + client: c, } - got := NewManager(nexus, client) + got := NewManager(nexus, c) assert.Equal(t, want.nexus, got.nexus) assert.Equal(t, want.client, got.client) } @@ -64,7 +65,7 @@ func TestManager_GetRequiredResources(t *testing.T) { // here we just want to check if they have been created and returned mgr := &Manager{ nexus: allDefaultsCommunityNexus, - client: test.NewFakeClientBuilder().Build(), + client: client.NewFakeClient(), log: logger.GetLoggerWithResource("test", allDefaultsCommunityNexus), } resources, err := mgr.GetRequiredResources() @@ -77,7 +78,7 @@ func TestManager_GetRequiredResources(t *testing.T) { func TestManager_GetDeployedResources(t *testing.T) { // first no deployed resources - fakeClient := test.NewFakeClientBuilder().Build() + fakeClient := client.NewFakeClient() mgr := &Manager{ nexus: allDefaultsCommunityNexus, client: fakeClient, diff --git a/controllers/nexus/resource/networking/manager.go b/controllers/nexus/resource/networking/manager.go index 75a9f88c..c97c51f0 100644 --- a/controllers/nexus/resource/networking/manager.go +++ b/controllers/nexus/resource/networking/manager.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/m88i/nexus-operator/api/v1alpha1" + operatorclient "github.com/m88i/nexus-operator/pkg/client" "github.com/m88i/nexus-operator/pkg/cluster/discovery" "github.com/m88i/nexus-operator/pkg/framework" "github.com/m88i/nexus-operator/pkg/framework/kind" @@ -149,7 +150,7 @@ func (m *Manager) createIngress() resource.KubernetesResource { // GetDeployedResources returns the networking resources deployed on the cluster func (m *Manager) GetDeployedResources() ([]resource.KubernetesResource, error) { - return framework.FetchDeployedResources(m.managedObjectsRef, framework.Key(m.nexus), m.client) + return operatorclient.FetchDeployedResources(m.managedObjectsRef, framework.Key(m.nexus), m.client) } // GetCustomComparator returns the custom comp function used to compare a networking resource. diff --git a/controllers/nexus/resource/networking/manager_test.go b/controllers/nexus/resource/networking/manager_test.go index 63fcba4a..8817960e 100644 --- a/controllers/nexus/resource/networking/manager_test.go +++ b/controllers/nexus/resource/networking/manager_test.go @@ -31,6 +31,7 @@ import ( "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/controllers/nexus/resource/deployment" + "github.com/m88i/nexus-operator/pkg/client" "github.com/m88i/nexus-operator/pkg/cluster/discovery" "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/logger" @@ -45,17 +46,17 @@ var nodePortNexus = &v1alpha1.Nexus{ } func TestNewManager(t *testing.T) { - k8sClient := test.NewFakeClientBuilder().Build() - k8sClientWithIngress := test.NewFakeClientBuilder().WithIngress().Build() - k8sClientWithLegacyIngress := test.NewFakeClientBuilder().WithLegacyIngress().Build() - ocpClient := test.NewFakeClientBuilder().OnOpenshift().Build() + k8sClient := discovery.NewFakeDiscBuilder().Build() + k8sClientWithIngress := discovery.NewFakeDiscBuilder().WithIngress().Build() + k8sClientWithLegacyIngress := discovery.NewFakeDiscBuilder().WithLegacyIngress().Build() + ocpClient := discovery.NewFakeDiscBuilder().OnOpenshift().Build() //default-setting logic is tested elsewhere //so here we just check if the resulting manager took in the arguments correctly tests := []struct { - name string - want *Manager - wantClient *test.FakeClient + name string + want *Manager + disc *discovery.FakeDisc }{ { "On Kubernetes with v1 Ingresses available", @@ -110,10 +111,11 @@ func TestNewManager(t *testing.T) { } for _, tt := range tests { - discovery.SetClient(tt.wantClient) - got, err := NewManager(nodePortNexus, tt.wantClient) + discovery.SetClient(tt.disc) + wantCli := client.NewFakeClient() + got, err := NewManager(nodePortNexus, wantCli) assert.NoError(t, err) - assert.Equal(t, tt.wantClient, got.client) + assert.Equal(t, wantCli, got.client) assert.Equal(t, tt.want.nexus, got.nexus) assert.Equal(t, tt.want.routeAvailable, got.routeAvailable) assert.Equal(t, tt.want.ingressAvailable, got.ingressAvailable) @@ -125,7 +127,7 @@ func TestNewManager(t *testing.T) { mockErrorMsg := "mock 500" k8sClient.SetMockErrorForOneRequest(errors.NewInternalError(fmt.Errorf(mockErrorMsg))) discovery.SetClient(k8sClient) - mgr, err := NewManager(nodePortNexus, k8sClient) + mgr, err := NewManager(nodePortNexus, client.NewFakeClient()) assert.Nil(t, mgr) assert.Contains(t, err.Error(), mockErrorMsg) } @@ -134,10 +136,11 @@ func TestManager_GetRequiredResources(t *testing.T) { // correctness of the generated resources is tested elsewhere // here we just want to check if they have been created and returned // first, let's test a Nexus which does not expose + discovery.SetClient(discovery.NewFakeDiscBuilder().Build()) nexus := &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{Expose: false}}} mgr := &Manager{ nexus: nexus, - client: test.NewFakeClientBuilder().Build(), + client: client.NewFakeClient(), log: logger.GetLoggerWithResource("test", nexus), } resources, err := mgr.GetRequiredResources() @@ -145,9 +148,10 @@ func TestManager_GetRequiredResources(t *testing.T) { assert.Nil(t, err) // now, let's use a route + discovery.SetClient(discovery.NewFakeDiscBuilder().OnOpenshift().Build()) mgr = &Manager{ nexus: routeNexus, - client: test.NewFakeClientBuilder().OnOpenshift().Build(), + client: client.NewFakeClient(), log: logger.GetLoggerWithResource("test", routeNexus), routeAvailable: true, } @@ -157,19 +161,21 @@ func TestManager_GetRequiredResources(t *testing.T) { assert.True(t, test.ContainsType(resources, reflect.TypeOf(&routev1.Route{}))) // still a route, but in a cluster without routes + discovery.SetClient(discovery.NewFakeDiscBuilder().Build()) mgr = &Manager{ nexus: routeNexus, log: logger.GetLoggerWithResource("test", routeNexus), - client: test.NewFakeClientBuilder().Build(), + client: client.NewFakeClient(), } resources, err = mgr.GetRequiredResources() assert.Nil(t, resources) assert.EqualError(t, err, fmt.Sprintf(resUnavailableFormat, "routes")) // now an ingress + discovery.SetClient(discovery.NewFakeDiscBuilder().WithIngress().Build()) mgr = &Manager{ nexus: nexusIngress, - client: test.NewFakeClientBuilder().WithLegacyIngress().Build(), + client: client.NewFakeClient(), log: logger.GetLoggerWithResource("test", nexusIngress), ingressAvailable: true, } @@ -179,10 +185,11 @@ func TestManager_GetRequiredResources(t *testing.T) { assert.True(t, test.ContainsType(resources, reflect.TypeOf(&networkingv1.Ingress{}))) // still an ingress, but in a cluster without ingresses + discovery.SetClient(discovery.NewFakeDiscBuilder().Build()) mgr = &Manager{ nexus: nexusIngress, log: logger.GetLoggerWithResource("test", nexusIngress), - client: test.NewFakeClientBuilder().Build(), + client: client.NewFakeClient(), } resources, err = mgr.GetRequiredResources() assert.Nil(t, resources) @@ -233,7 +240,7 @@ func TestManager_createIngress(t *testing.T) { func TestManager_GetDeployedResources(t *testing.T) { // first with no deployed resources - fakeClient := test.NewFakeClientBuilder().WithIngress().OnOpenshift().Build() + fakeClient := client.NewFakeClient() mgr := &Manager{ nexus: nodePortNexus, client: fakeClient, diff --git a/controllers/nexus/resource/persistence/manager.go b/controllers/nexus/resource/persistence/manager.go index 1b879627..b5ab0cb4 100644 --- a/controllers/nexus/resource/persistence/manager.go +++ b/controllers/nexus/resource/persistence/manager.go @@ -24,6 +24,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/m88i/nexus-operator/api/v1alpha1" + operatorclient "github.com/m88i/nexus-operator/pkg/client" "github.com/m88i/nexus-operator/pkg/framework" "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/logger" @@ -69,7 +70,7 @@ func (m *Manager) GetRequiredResources() ([]resource.KubernetesResource, error) func (m *Manager) GetDeployedResources() ([]resource.KubernetesResource, error) { var resources []resource.KubernetesResource for resType, resRef := range managedObjectsRef { - if err := framework.Fetch(m.client, framework.Key(m.nexus), resRef, resType); err == nil { + if err := operatorclient.Fetch(m.client, framework.Key(m.nexus), resRef, resType); err == nil { resources = append(resources, resRef) } else if !errors.IsNotFound(err) { return nil, fmt.Errorf("could not fetch %s (%s/%s): %v", resType, m.nexus.Namespace, m.nexus.Name, err) diff --git a/controllers/nexus/resource/persistence/manager_test.go b/controllers/nexus/resource/persistence/manager_test.go index 9c333e06..ac0d4da3 100644 --- a/controllers/nexus/resource/persistence/manager_test.go +++ b/controllers/nexus/resource/persistence/manager_test.go @@ -26,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/m88i/nexus-operator/api/v1alpha1" + "github.com/m88i/nexus-operator/pkg/client" "github.com/m88i/nexus-operator/pkg/logger" "github.com/m88i/nexus-operator/pkg/test" ) @@ -36,12 +37,12 @@ func TestNewManager(t *testing.T) { // default-setting logic is tested elsewhere // so here we just check if the resulting manager took in the arguments correctly nexus := baseNexus - client := test.NewFakeClientBuilder().Build() + c := client.NewFakeClient() want := &Manager{ nexus: nexus, - client: client, + client: c, } - got := NewManager(nexus, client) + got := NewManager(nexus, c) assert.Equal(t, want.nexus, got.nexus) assert.Equal(t, want.client, got.client) } @@ -51,7 +52,7 @@ func TestManager_GetRequiredResources(t *testing.T) { // here we just want to check if they have been created and returned mgr := &Manager{ nexus: baseNexus.DeepCopy(), - client: test.NewFakeClientBuilder().Build(), + client: client.NewFakeClient(), log: logger.GetLoggerWithResource("test", baseNexus), } @@ -74,7 +75,7 @@ func TestManager_GetRequiredResources(t *testing.T) { func TestManager_GetDeployedResources(t *testing.T) { // first with no deployed resources - fakeClient := test.NewFakeClientBuilder().Build() + fakeClient := client.NewFakeClient() mgr := &Manager{ nexus: baseNexus, client: fakeClient, diff --git a/controllers/nexus/resource/security/manager.go b/controllers/nexus/resource/security/manager.go index dd0217ff..861db2c8 100644 --- a/controllers/nexus/resource/security/manager.go +++ b/controllers/nexus/resource/security/manager.go @@ -24,6 +24,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/m88i/nexus-operator/api/v1alpha1" + operatorclient "github.com/m88i/nexus-operator/pkg/client" "github.com/m88i/nexus-operator/pkg/framework" "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/logger" @@ -62,7 +63,7 @@ func (m *Manager) GetRequiredResources() ([]resource.KubernetesResource, error) func (m *Manager) GetDeployedResources() ([]resource.KubernetesResource, error) { var resources []resource.KubernetesResource for resType, resRef := range managedObjectsRef { - if err := framework.Fetch(m.client, framework.Key(m.nexus), resRef, resType); err == nil { + if err := operatorclient.Fetch(m.client, framework.Key(m.nexus), resRef, resType); err == nil { resources = append(resources, resRef) } else if !errors.IsNotFound(err) { return nil, fmt.Errorf("could not fetch %s (%s/%s): %v", resType, m.nexus.Namespace, m.nexus.Name, err) diff --git a/controllers/nexus/resource/security/manager_test.go b/controllers/nexus/resource/security/manager_test.go index 1a70b321..aa1f54c1 100644 --- a/controllers/nexus/resource/security/manager_test.go +++ b/controllers/nexus/resource/security/manager_test.go @@ -20,16 +20,16 @@ import ( "reflect" "testing" - "github.com/m88i/nexus-operator/pkg/framework/kind" - "github.com/m88i/nexus-operator/pkg/logger" - "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/m88i/nexus-operator/api/v1alpha1" + "github.com/m88i/nexus-operator/pkg/client" "github.com/m88i/nexus-operator/pkg/framework" + "github.com/m88i/nexus-operator/pkg/framework/kind" + "github.com/m88i/nexus-operator/pkg/logger" "github.com/m88i/nexus-operator/pkg/test" ) @@ -39,12 +39,12 @@ func TestNewManager(t *testing.T) { // default-setting logic is tested elsewhere // so here we just check if the resulting manager took in the arguments correctly nexus := baseNexus - client := test.NewFakeClientBuilder().Build() + cli := client.NewFakeClient() want := &Manager{ nexus: nexus, - client: client, + client: cli, } - got := NewManager(nexus, client) + got := NewManager(nexus, cli) assert.Equal(t, want.nexus, got.nexus) assert.Equal(t, want.client, got.client) } @@ -54,7 +54,7 @@ func TestManager_GetRequiredResources(t *testing.T) { // here we just want to check if they have been created and returned mgr := &Manager{ nexus: baseNexus.DeepCopy(), - client: test.NewFakeClientBuilder().Build(), + client: client.NewFakeClient(), log: logger.GetLoggerWithResource("test", baseNexus), } @@ -69,7 +69,7 @@ func TestManager_GetRequiredResources(t *testing.T) { func TestManager_GetDeployedResources(t *testing.T) { // first with no deployed resources - fakeClient := test.NewFakeClientBuilder().Build() + fakeClient := client.NewFakeClient() mgr := &Manager{ nexus: baseNexus, client: fakeClient, @@ -99,17 +99,17 @@ func TestManager_GetDeployedResources(t *testing.T) { func TestManager_getDeployedSvcAccnt(t *testing.T) { mgr := &Manager{ nexus: baseNexus, - client: test.NewFakeClientBuilder().Build(), + client: client.NewFakeClient(), } // first, test without creating the svcAccnt - err := framework.Fetch(mgr.client, framework.Key(mgr.nexus), managedObjectsRef[kind.SvcAccountKind], kind.SvcAccountKind) + err := client.Fetch(mgr.client, framework.Key(mgr.nexus), managedObjectsRef[kind.SvcAccountKind], kind.SvcAccountKind) assert.True(t, errors.IsNotFound(err)) // now test after creating the svcAccnt svcAccnt := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: mgr.nexus.Name, Namespace: mgr.nexus.Namespace}} assert.NoError(t, mgr.client.Create(ctx.TODO(), svcAccnt)) - err = framework.Fetch(mgr.client, framework.Key(svcAccnt), svcAccnt, kind.SvcAccountKind) + err = client.Fetch(mgr.client, framework.Key(svcAccnt), svcAccnt, kind.SvcAccountKind) assert.NotNil(t, svcAccnt) assert.NoError(t, err) } diff --git a/controllers/nexus/server/nexus_test.go b/controllers/nexus/server/nexus_test.go index bb97becf..9a4e2384 100644 --- a/controllers/nexus/server/nexus_test.go +++ b/controllers/nexus/server/nexus_test.go @@ -31,7 +31,7 @@ import ( "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/controllers/nexus/resource/meta" - "github.com/m88i/nexus-operator/pkg/test" + operatorclient "github.com/m88i/nexus-operator/pkg/client" ) // createNewServerAndKubeCli creates a new fake server and kubernetes fake client to be used in tests for this package; @@ -39,9 +39,7 @@ import ( func createNewServerAndKubeCli(t *testing.T, objects ...runtime.Object) (*server, client.Client) { nexusInstance := &v1alpha1.Nexus{ObjectMeta: v1.ObjectMeta{Name: "nexus3", Namespace: t.Name()}} objects = append(objects, nexusInstance) - cli := test.NewFakeClientBuilder( - objects...). - Build() + cli := operatorclient.NewFakeClient(objects...) server := &server{ nexus: nexusInstance, k8sclient: cli, @@ -78,7 +76,7 @@ func Test_server_getNexusEndpoint(t *testing.T) { SessionAffinity: corev1.ServiceAffinityNone, }, } - cli := test.NewFakeClientBuilder(instance, svc).Build() + cli := operatorclient.NewFakeClient(svc, instance) s := server{ nexus: instance, k8sclient: cli, @@ -96,7 +94,7 @@ func Test_server_getNexusEndpointNoURL(t *testing.T) { Spec: v1alpha1.NexusSpec{}, ObjectMeta: v1.ObjectMeta{Name: "nexus3", Namespace: t.Name()}, } - cli := test.NewFakeClientBuilder(instance).Build() + cli := operatorclient.NewFakeClient(instance) s := server{ nexus: instance, k8sclient: cli, @@ -135,7 +133,7 @@ func Test_HandleServerOperationsNoFake(t *testing.T) { Spec: v1alpha1.NexusSpec{}, ObjectMeta: v1.ObjectMeta{Name: "nexus3", Namespace: t.Name()}, } - cli := test.NewFakeClientBuilder(instance).Build() + cli := operatorclient.NewFakeClient(instance) status, err := HandleServerOperations(instance, cli) assert.NoError(t, err) @@ -169,7 +167,7 @@ func Test_handleServerOperations(t *testing.T) { SessionAffinity: corev1.ServiceAffinityNone, }, } - cli := test.NewFakeClientBuilder(instance, svc, &corev1.Secret{ObjectMeta: v1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace}}).Build() + cli := operatorclient.NewFakeClient(instance, svc, &corev1.Secret{ObjectMeta: v1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace}}) status, err := handleServerOperations(instance, cli, nexusAPIFakeBuilder) assert.NoError(t, err) assert.NotNil(t, status) @@ -190,7 +188,7 @@ func Test_handleServerOperationsNoEndpoint(t *testing.T) { }, }, } - cli := test.NewFakeClientBuilder(instance).Build() + cli := operatorclient.NewFakeClient(instance) status, err := handleServerOperations(instance, cli, nexusAPIFakeBuilder) assert.NoError(t, err) assert.NotNil(t, status) diff --git a/controllers/nexus/server/users.go b/controllers/nexus/server/users.go index 0b1e4ad0..1c60f9d5 100644 --- a/controllers/nexus/server/users.go +++ b/controllers/nexus/server/users.go @@ -21,6 +21,7 @@ import ( "github.com/m88i/aicura/nexus" corev1 "k8s.io/api/core/v1" + "github.com/m88i/nexus-operator/pkg/client" "github.com/m88i/nexus-operator/pkg/framework" "github.com/m88i/nexus-operator/pkg/framework/kind" ) @@ -108,7 +109,7 @@ func (u *userOperation) createOperatorUserIfNotExists() (*nexus.User, error) { func (u *userOperation) storeOperatorUserCredentials(user *nexus.User) error { secret := &corev1.Secret{} log.Debug("Attempt to store operator user credentials into Secret") - if err := framework.Fetch(u.k8sclient, framework.Key(u.nexus), secret, kind.SecretKind); err != nil { + if err := client.Fetch(u.k8sclient, framework.Key(u.nexus), secret, kind.SecretKind); err != nil { return err } if secret.StringData == nil { @@ -125,7 +126,7 @@ func (u *userOperation) storeOperatorUserCredentials(user *nexus.User) error { func (u *userOperation) getOperatorUserCredentials() (user, password string, err error) { secret := &corev1.Secret{} - if err := framework.Fetch(u.k8sclient, framework.Key(u.nexus), secret, kind.SecretKind); err != nil { + if err := client.Fetch(u.k8sclient, framework.Key(u.nexus), secret, kind.SecretKind); err != nil { return "", "", err } return string(secret.Data[SecretKeyUsername]), string(secret.Data[SecretKeyPassword]), nil diff --git a/controllers/nexus/update/events_test.go b/controllers/nexus/update/events_test.go index 92a30c75..57631c21 100644 --- a/controllers/nexus/update/events_test.go +++ b/controllers/nexus/update/events_test.go @@ -24,24 +24,24 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/m88i/nexus-operator/api/v1alpha1" - "github.com/m88i/nexus-operator/pkg/test" + "github.com/m88i/nexus-operator/pkg/client" ) func TestCreateUpdateSuccessEvent(t *testing.T) { nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus", Namespace: "test"}} - client := test.NewFakeClientBuilder().Build() + cli := client.NewFakeClient() tag := "3.25.0" // first, let's test a failure - client.SetMockErrorForOneRequest(fmt.Errorf("mock err")) - createUpdateSuccessEvent(nexus, client.Scheme(), client, tag) + cli.SetMockErrorForOneRequest(fmt.Errorf("mock err")) + createUpdateSuccessEvent(nexus, cli.Scheme(), cli, tag) eventList := &corev1.EventList{} - _ = client.List(ctx.TODO(), eventList) + _ = cli.List(ctx.TODO(), eventList) assert.Len(t, eventList.Items, 0) // now a successful one - createUpdateSuccessEvent(nexus, client.Scheme(), client, tag) - _ = client.List(ctx.TODO(), eventList) + createUpdateSuccessEvent(nexus, cli.Scheme(), cli, tag) + _ = cli.List(ctx.TODO(), eventList) assert.Len(t, eventList.Items, 1) event := eventList.Items[0] assert.Equal(t, successfulUpdateReason, event.Reason) @@ -49,19 +49,19 @@ func TestCreateUpdateSuccessEvent(t *testing.T) { func TestCreateUpdateFailureEvent(t *testing.T) { nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus", Namespace: "test"}} - client := test.NewFakeClientBuilder().Build() + cli := client.NewFakeClient() tag := "3.25.0" // first, let's test a failure - client.SetMockErrorForOneRequest(fmt.Errorf("mock err")) - createUpdateFailureEvent(nexus, client.Scheme(), client, tag) + cli.SetMockErrorForOneRequest(fmt.Errorf("mock err")) + createUpdateFailureEvent(nexus, cli.Scheme(), cli, tag) eventList := &corev1.EventList{} - _ = client.List(ctx.TODO(), eventList) + _ = cli.List(ctx.TODO(), eventList) assert.Len(t, eventList.Items, 0) // now a successful one - createUpdateFailureEvent(nexus, client.Scheme(), client, tag) - _ = client.List(ctx.TODO(), eventList) + createUpdateFailureEvent(nexus, cli.Scheme(), cli, tag) + _ = cli.List(ctx.TODO(), eventList) assert.Len(t, eventList.Items, 1) event := eventList.Items[0] assert.Equal(t, failedUpdateReason, event.Reason) diff --git a/controllers/nexus/update/monitor_test.go b/controllers/nexus/update/monitor_test.go index fd9e4a52..7262a9bf 100644 --- a/controllers/nexus/update/monitor_test.go +++ b/controllers/nexus/update/monitor_test.go @@ -24,6 +24,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/m88i/nexus-operator/api/v1alpha1" + "github.com/m88i/nexus-operator/pkg/client" "github.com/m88i/nexus-operator/pkg/test" ) @@ -43,7 +44,7 @@ func TestMonitorUpdate(t *testing.T) { Status: v1alpha1.NexusStatus{}, Spec: v1alpha1.NexusSpec{Image: image}, } - c := test.NewFakeClientBuilder(nexus).Build() + c := client.NewFakeClient(nexus) // Not in an update and will not start one deployedDep := baseDeployment.DeepCopy() diff --git a/controllers/nexus_controller_test.go b/controllers/nexus_controller_test.go index ba38cb16..e3964ffd 100644 --- a/controllers/nexus_controller_test.go +++ b/controllers/nexus_controller_test.go @@ -55,6 +55,7 @@ var _ = Describe("Nexus Controller", func() { Spec: v1alpha1.NexusSpec{ Replicas: 1, UseRedHatImage: false, + Image: v1alpha1.NexusCommunityImage, Resources: v1.ResourceRequirements{ Limits: map[v1.ResourceName]resource.Quantity{ v1.ResourceCPU: resource.MustParse(cpu), @@ -67,6 +68,8 @@ var _ = Describe("Nexus Controller", func() { ExposeAs: v1alpha1.NodePortExposeType, NodePort: exposedPort, }, + LivenessProbe: v1alpha1.DefaultProbe, + ReadinessProbe: v1alpha1.DefaultProbe, }, } ) diff --git a/hack/go-fmt.sh b/hack/go-fmt.sh index b564bdbc..70eb86f3 100755 --- a/hack/go-fmt.sh +++ b/hack/go-fmt.sh @@ -19,5 +19,5 @@ go mod tidy gofmt -s -l -w . # get the goimports binary -command -v goimports >/dev/null || go build -o "${GOPATH}"/bin/goimports golang.org/x/tools/cmd/goimports -goimports -local github.com/m88i/nexus-operator -l -w . +command -v ./bin/goimports >/dev/null || go build -o ./bin/goimports golang.org/x/tools/cmd/goimports +./bin/goimports -local github.com/m88i/nexus-operator -l -w . diff --git a/pkg/test/client.go b/pkg/client/fake.go similarity index 94% rename from pkg/test/client.go rename to pkg/client/fake.go index 63aff009..34bb842d 100644 --- a/pkg/test/client.go +++ b/pkg/client/fake.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package test +package client import ( "context" @@ -32,12 +32,14 @@ type FakeClient struct { client client.Client mockErr error shouldClearError bool + scheme *runtime.Scheme } // NewFakeClient returns a FakeClient. // You may initialize it with a slice of runtime.Object. func NewFakeClient(initObjs ...runtime.Object) *FakeClient { - return &FakeClient{client: fake.NewFakeClientWithScheme(scheme(), initObjs...)} + s := scheme() + return &FakeClient{client: fake.NewFakeClientWithScheme(s, initObjs...), scheme: s} } func scheme() *runtime.Scheme { @@ -141,3 +143,7 @@ func (c *FakeClient) DeleteAllOf(ctx context.Context, obj runtime.Object, opts . func (c FakeClient) Status() client.StatusWriter { return c.client.Status() } + +func (c FakeClient) Scheme() *runtime.Scheme { + return c.scheme +} diff --git a/pkg/test/client_test.go b/pkg/client/fake_test.go similarity index 66% rename from pkg/test/client_test.go rename to pkg/client/fake_test.go index b012848d..a1522858 100644 --- a/pkg/test/client_test.go +++ b/pkg/client/fake_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package test +package client import ( ctx "context" @@ -24,8 +24,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/pkg/framework" + "github.com/m88i/nexus-operator/pkg/test" ) const testErrorMsg = "test" @@ -34,7 +34,7 @@ func TestNewFakeClient(t *testing.T) { dep := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "dep", Namespace: t.Name()}} key := framework.Key(dep) c := NewFakeClient(dep) - assert.False(t, IsInterfaceValueNil(c.client)) // make sure there actually is a client + assert.False(t, test.IsInterfaceValueNil(c.client)) // make sure there actually is a client assert.NoError(t, c.Get(ctx.Background(), key, dep)) } @@ -42,11 +42,11 @@ func TestFakeClient_Get(t *testing.T) { c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) - err := c.Get(ctx.TODO(), client.ObjectKey{}, &v1alpha1.Nexus{}) + err := c.Get(ctx.TODO(), client.ObjectKey{}, &appsv1.Deployment{}) assert.Equal(t, mockErr, err) - want := c.client.Get(ctx.TODO(), client.ObjectKey{}, &v1alpha1.Nexus{}) - got := c.Get(ctx.TODO(), client.ObjectKey{}, &v1alpha1.Nexus{}) + want := c.client.Get(ctx.TODO(), client.ObjectKey{}, &appsv1.Deployment{}) + got := c.Get(ctx.TODO(), client.ObjectKey{}, &appsv1.Deployment{}) assert.Equal(t, want, got) assert.NotEqual(t, got, mockErr) } @@ -55,11 +55,11 @@ func TestFakeClient_List(t *testing.T) { c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) - err := c.List(ctx.TODO(), &v1alpha1.NexusList{}) + err := c.List(ctx.TODO(), &appsv1.DeploymentList{}) assert.Equal(t, mockErr, err) - want := c.client.List(ctx.TODO(), &v1alpha1.NexusList{}) - got := c.List(ctx.TODO(), &v1alpha1.NexusList{}) + want := c.client.List(ctx.TODO(), &appsv1.DeploymentList{}) + got := c.List(ctx.TODO(), &appsv1.DeploymentList{}) assert.Equal(t, want, got) assert.NotEqual(t, got, mockErr) } @@ -68,11 +68,11 @@ func TestFakeClient_Create(t *testing.T) { c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) - err := c.Create(ctx.TODO(), &v1alpha1.Nexus{}) + err := c.Create(ctx.TODO(), &appsv1.Deployment{}) assert.Equal(t, mockErr, err) - want := c.client.Create(ctx.TODO(), &v1alpha1.Nexus{}) - got := c.Create(ctx.TODO(), &v1alpha1.Nexus{}) + want := c.client.Create(ctx.TODO(), &appsv1.Deployment{}) + got := c.Create(ctx.TODO(), &appsv1.Deployment{}) assert.Equal(t, want, got) assert.NotEqual(t, got, mockErr) } @@ -81,11 +81,11 @@ func TestFakeClient_Delete(t *testing.T) { c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) - err := c.Delete(ctx.TODO(), &v1alpha1.Nexus{}) + err := c.Delete(ctx.TODO(), &appsv1.Deployment{}) assert.Equal(t, mockErr, err) - want := c.client.Delete(ctx.TODO(), &v1alpha1.Nexus{}) - got := c.Delete(ctx.TODO(), &v1alpha1.Nexus{}) + want := c.client.Delete(ctx.TODO(), &appsv1.Deployment{}) + got := c.Delete(ctx.TODO(), &appsv1.Deployment{}) assert.Equal(t, want, got) assert.NotEqual(t, got, mockErr) } @@ -94,11 +94,11 @@ func TestFakeClient_Update(t *testing.T) { c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) - err := c.Update(ctx.TODO(), &v1alpha1.Nexus{}) + err := c.Update(ctx.TODO(), &appsv1.Deployment{}) assert.Equal(t, mockErr, err) - want := c.client.Update(ctx.TODO(), &v1alpha1.Nexus{}) - got := c.Update(ctx.TODO(), &v1alpha1.Nexus{}) + want := c.client.Update(ctx.TODO(), &appsv1.Deployment{}) + got := c.Update(ctx.TODO(), &appsv1.Deployment{}) assert.Equal(t, want, got) assert.NotEqual(t, got, mockErr) } @@ -107,11 +107,11 @@ func TestFakeClient_Patch(t *testing.T) { c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) - err := c.Patch(ctx.TODO(), &v1alpha1.Nexus{}, client.MergeFrom(&v1alpha1.Nexus{})) + err := c.Patch(ctx.TODO(), &appsv1.Deployment{}, client.MergeFrom(&appsv1.Deployment{})) assert.Equal(t, mockErr, err) - want := c.Patch(ctx.TODO(), &v1alpha1.Nexus{}, client.MergeFrom(&v1alpha1.Nexus{})) - got := c.Patch(ctx.TODO(), &v1alpha1.Nexus{}, client.MergeFrom(&v1alpha1.Nexus{})) + want := c.Patch(ctx.TODO(), &appsv1.Deployment{}, client.MergeFrom(&appsv1.Deployment{})) + got := c.Patch(ctx.TODO(), &appsv1.Deployment{}, client.MergeFrom(&appsv1.Deployment{})) assert.Equal(t, want, got) assert.NotEqual(t, got, mockErr) } @@ -120,11 +120,11 @@ func TestFakeClient_DeleteAllOf(t *testing.T) { c := NewFakeClient() mockErr := fmt.Errorf(testErrorMsg) c.SetMockErrorForOneRequest(mockErr) - err := c.DeleteAllOf(ctx.TODO(), &v1alpha1.Nexus{}) + err := c.DeleteAllOf(ctx.TODO(), &appsv1.Deployment{}) assert.Equal(t, mockErr, err) - want := c.client.DeleteAllOf(ctx.TODO(), &v1alpha1.Nexus{}) - got := c.DeleteAllOf(ctx.TODO(), &v1alpha1.Nexus{}) + want := c.client.DeleteAllOf(ctx.TODO(), &appsv1.Deployment{}) + got := c.DeleteAllOf(ctx.TODO(), &appsv1.Deployment{}) assert.Equal(t, want, got) assert.NotEqual(t, got, mockErr) } diff --git a/pkg/framework/fetcher.go b/pkg/client/fetcher.go similarity index 95% rename from pkg/framework/fetcher.go rename to pkg/client/fetcher.go index 95f6313a..e65ff88f 100644 --- a/pkg/framework/fetcher.go +++ b/pkg/client/fetcher.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package framework +package client import ( ctx "context" @@ -22,8 +22,12 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/m88i/nexus-operator/pkg/logger" ) +var log = logger.GetLogger("client") + // FetchDeployedResources fetches deployed resources whose Kind is present in "managedObjectsRef" func FetchDeployedResources(managedObjectsRef map[string]resource.KubernetesResource, key types.NamespacedName, cli client.Client) ([]resource.KubernetesResource, error) { var resources []resource.KubernetesResource diff --git a/pkg/framework/fetcher_test.go b/pkg/client/fetcher_test.go similarity index 71% rename from pkg/framework/fetcher_test.go rename to pkg/client/fetcher_test.go index d3343aaf..449e7a51 100644 --- a/pkg/framework/fetcher_test.go +++ b/pkg/client/fetcher_test.go @@ -12,12 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -package framework +package client import ( goerrors "errors" "testing" + "github.com/m88i/nexus-operator/pkg/framework" + "github.com/RHsyseng/operator-utils/pkg/resource" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" @@ -25,53 +27,50 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/pkg/framework/kind" - "github.com/m88i/nexus-operator/pkg/test" ) func TestFetchDeployedResources(t *testing.T) { - nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus-test", Namespace: t.Name()}} - deployment := &appsv1.Deployment{ObjectMeta: nexus.ObjectMeta} - service := &corev1.Service{ObjectMeta: nexus.ObjectMeta} + meta := metav1.ObjectMeta{Name: "test", Namespace: t.Name()} + deployment := &appsv1.Deployment{ObjectMeta: meta} + service := &corev1.Service{ObjectMeta: meta} managedObjectsRef := map[string]resource.KubernetesResource{ kind.ServiceKind: &corev1.Service{}, kind.DeploymentKind: &appsv1.Deployment{}, // we won't have a SA, but this is useful to test no error is triggered when a resource isn't found kind.SvcAccountKind: &corev1.ServiceAccount{}, } - cli := test.NewFakeClientBuilder(deployment, service).Build() - - gotResources, err := FetchDeployedResources(managedObjectsRef, Key(nexus), cli) + cli := NewFakeClient(deployment, service) + gotResources, err := FetchDeployedResources(managedObjectsRef, framework.Key(deployment), cli) assert.Nil(t, err) assert.Len(t, gotResources, 2) } func TestFetchDeployedResourcesFailure(t *testing.T) { - nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus-test", Namespace: t.Name()}} + dep := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "dep-test", Namespace: t.Name()}} // managedObjectsRef cannot be empty in order to raise error, the content is irrelevant though managedObjectsRef := map[string]resource.KubernetesResource{kind.DeploymentKind: &appsv1.Deployment{}} - cli := test.NewFakeClientBuilder().Build() + cli := NewFakeClient() mockErrorMsg := "mock error" cli.SetMockError(goerrors.New(mockErrorMsg)) - _, err := FetchDeployedResources(managedObjectsRef, Key(nexus), cli) + _, err := FetchDeployedResources(managedObjectsRef, framework.Key(dep), cli) assert.Contains(t, err.Error(), mockErrorMsg) } func TestFetch(t *testing.T) { deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "deployment", Namespace: t.Name()}} - cli := test.NewFakeClientBuilder(deployment).Build() - err := Fetch(cli, Key(deployment), deployment, kind.DeploymentKind) + cli := NewFakeClient(deployment) + err := Fetch(cli, framework.Key(deployment), deployment, kind.DeploymentKind) assert.NoError(t, err) } func TestNotFoundFetch(t *testing.T) { deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "deployment", Namespace: t.Name()}} - cli := test.NewFakeClientBuilder().Build() - err := Fetch(cli, Key(deployment), deployment, kind.DeploymentKind) + cli := NewFakeClient() + err := Fetch(cli, framework.Key(deployment), deployment, kind.DeploymentKind) assert.Error(t, err) assert.True(t, errors.IsNotFound(err)) } diff --git a/pkg/cluster/discovery/fake.go b/pkg/cluster/discovery/fake.go index db9f5ad0..acbfbca4 100644 --- a/pkg/cluster/discovery/fake.go +++ b/pkg/cluster/discovery/fake.go @@ -1,3 +1,17 @@ +// Copyright 2020 Nexus Operator and/or its 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 discovery import ( diff --git a/pkg/cluster/discovery/fake_test.go b/pkg/cluster/discovery/fake_test.go index 74eae3be..746a8856 100644 --- a/pkg/cluster/discovery/fake_test.go +++ b/pkg/cluster/discovery/fake_test.go @@ -1,3 +1,17 @@ +// Copyright 2020 Nexus Operator and/or its 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 discovery import ( diff --git a/pkg/cluster/discovery/kubernetes.go b/pkg/cluster/discovery/kubernetes.go index a36d571f..7e535d93 100644 --- a/pkg/cluster/discovery/kubernetes.go +++ b/pkg/cluster/discovery/kubernetes.go @@ -30,3 +30,17 @@ func IsIngressAvailable() (bool, error) { func IsLegacyIngressAvailable() (bool, error) { return hasGroupVersionKind(networkingv1beta1.SchemeGroupVersion.Group, networkingv1beta1.SchemeGroupVersion.Version, kind.IngressKind) } + +// IsAnyIngressAvailable checks if either v1 or v1beta1 ingresses are available +func IsAnyIngressAvailable() (bool, error) { + legacyIngressAvailable, errLegacy := IsLegacyIngressAvailable() + ingressAvailable, err := IsIngressAvailable() + + // both calls failed, let's error out + if errLegacy != nil && err != nil { + return false, errLegacy + } + + // at least one call didn't fail + return legacyIngressAvailable || ingressAvailable, nil +} diff --git a/pkg/cluster/discovery/kubernetes_test.go b/pkg/cluster/discovery/kubernetes_test.go index 1b3dc4d4..4507e7be 100644 --- a/pkg/cluster/discovery/kubernetes_test.go +++ b/pkg/cluster/discovery/kubernetes_test.go @@ -15,8 +15,11 @@ package discovery import ( + "errors" "testing" + "k8s.io/client-go/discovery" + "github.com/stretchr/testify/assert" ) @@ -43,3 +46,65 @@ func TestIsLegacyIngressAvailable(t *testing.T) { assert.Nil(t, err) assert.True(t, ingressAvailable) } + +func TestIsAnyIngressAvailable(t *testing.T) { + testCases := []struct { + name string + disc discovery.DiscoveryInterface + wantIngressAvailable bool + wantError bool + }{ + { + "no ingresses available", + NewFakeDiscBuilder().Build(), + false, + false, + }, + { + "v1beta1 ingress only available", + NewFakeDiscBuilder().WithLegacyIngress().Build(), + true, + false, + }, + { + "v1 ingress available", + NewFakeDiscBuilder().WithIngress().Build(), + true, + false, + }, + { + "both ingresses available", + NewFakeDiscBuilder().WithIngress().WithLegacyIngress().Build(), + true, + false, + }, + { + "both ingresses available. One call fails, the other succeeds", + func() discovery.DiscoveryInterface { + d := NewFakeDiscBuilder().WithIngress().WithLegacyIngress().Build() + d.SetMockErrorForOneRequest(errors.New("mock err")) + return d + }(), + true, + false, + }, + { + "both ingresses available. Both calls fail", + func() discovery.DiscoveryInterface { + d := NewFakeDiscBuilder().WithIngress().WithLegacyIngress().Build() + d.SetMockError(errors.New("mock err")) + return d + }(), + false, + true, + }, + } + + for _, tc := range testCases { + SetClient(tc.disc) + ingressAvailable, err := IsAnyIngressAvailable() + if tc.wantIngressAvailable != ingressAvailable || tc.wantError != (err != nil) { + t.Errorf("%s\nwantIngressAvailable: %v\t got: %v\nwantError: %v\tgotError: %#v\n", tc.name, tc.wantIngressAvailable, ingressAvailable, tc.wantError, err) + } + } +} diff --git a/pkg/cluster/kubernetes/events_test.go b/pkg/cluster/kubernetes/events_test.go index ec30c1ad..a4cbcfdb 100644 --- a/pkg/cluster/kubernetes/events_test.go +++ b/pkg/cluster/kubernetes/events_test.go @@ -25,20 +25,20 @@ import ( "k8s.io/apimachinery/pkg/runtime" "github.com/m88i/nexus-operator/api/v1alpha1" - "github.com/m88i/nexus-operator/pkg/test" + "github.com/m88i/nexus-operator/pkg/client" ) func TestRaiseInfoEventf(t *testing.T) { nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus", Namespace: "test"}} - client := test.NewFakeClientBuilder().Build() + cli := client.NewFakeClient() reason := "test-reason" format := "%s %s" message := "test-message" extraArg := "extra" - assert.NoError(t, RaiseInfoEventf(nexus, client.Scheme(), client, reason, format, message, extraArg)) + assert.NoError(t, RaiseInfoEventf(nexus, cli.Scheme(), cli, reason, format, message, extraArg)) eventList := &corev1.EventList{} - assert.NoError(t, client.List(ctx.TODO(), eventList)) + assert.NoError(t, cli.List(ctx.TODO(), eventList)) event := eventList.Items[0] assert.Equal(t, nexus.Name, event.Source.Component) assert.Equal(t, reason, event.Reason) @@ -48,15 +48,15 @@ func TestRaiseInfoEventf(t *testing.T) { func TestRaiseWarnEventf(t *testing.T) { nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus", Namespace: "test"}} - client := test.NewFakeClientBuilder().Build() + cli := client.NewFakeClient() reason := "test-reason" format := "%s %s" message := "test-message" extraArg := "extra" - assert.NoError(t, RaiseWarnEventf(nexus, client.Scheme(), client, reason, format, message, extraArg)) + assert.NoError(t, RaiseWarnEventf(nexus, cli.Scheme(), cli, reason, format, message, extraArg)) eventList := &corev1.EventList{} - assert.NoError(t, client.List(ctx.TODO(), eventList)) + assert.NoError(t, cli.List(ctx.TODO(), eventList)) event := eventList.Items[0] assert.Equal(t, nexus.Name, event.Source.Component) assert.Equal(t, reason, event.Reason) @@ -66,20 +66,20 @@ func TestRaiseWarnEventf(t *testing.T) { func TestServerFailure(t *testing.T) { nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus", Namespace: "test"}} - client := test.NewFakeClientBuilder().Build() + cli := client.NewFakeClient() reason := "test-reason" message := "test-message" - client.SetMockErrorForOneRequest(fmt.Errorf("mock-error")) - assert.Error(t, RaiseInfoEventf(nexus, client.Scheme(), client, reason, message)) + cli.SetMockErrorForOneRequest(fmt.Errorf("mock-error")) + assert.Error(t, RaiseInfoEventf(nexus, cli.Scheme(), cli, reason, message)) } func TestReferenceFailure(t *testing.T) { nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus", Namespace: "test"}} - client := test.NewFakeClientBuilder().Build() + cli := client.NewFakeClient() reason := "test-reason" message := "test-message" // let's pass in the default scheme - assert.Error(t, RaiseInfoEventf(nexus, runtime.NewScheme(), client, reason, message)) + assert.Error(t, RaiseInfoEventf(nexus, runtime.NewScheme(), cli, reason, message)) } diff --git a/pkg/test/utils_test.go b/pkg/test/utils_test.go index e11900fd..f98a723e 100644 --- a/pkg/test/utils_test.go +++ b/pkg/test/utils_test.go @@ -21,6 +21,8 @@ import ( "github.com/RHsyseng/operator-utils/pkg/resource" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" + + "github.com/m88i/nexus-operator/pkg/client" ) func TestContainsType(t *testing.T) { @@ -32,7 +34,7 @@ func TestContainsType(t *testing.T) { func TestEventExists(t *testing.T) { testReason := "reason" testEvent := &corev1.Event{Reason: testReason} - c := NewFakeClient(testEvent) + c := client.NewFakeClient(testEvent) assert.False(t, EventExists(c, "some other reason")) assert.True(t, EventExists(c, testReason)) From 63e29708be5ce88195d8356f85c5d05ad2d4fbc3 Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Tue, 22 Dec 2020 16:58:23 -0300 Subject: [PATCH 04/10] Enable kustomization for webhooks and cert-manager --- Makefile | 2 +- bundle/manifests/apps.m88i.io_nexus.yaml | 11 ++ .../nexus-operator.clusterserviceversion.yaml | 48 ++++++++ config/crd/bases/apps.m88i.io_nexus.yaml | 1 + config/crd/kustomization.yaml | 4 +- config/crd/patches/webhook_in_nexus.yaml | 1 + config/default/kustomization.yaml | 62 +++++----- nexus-operator.yaml | 112 ++++++++++++++++++ 8 files changed, 207 insertions(+), 34 deletions(-) diff --git a/Makefile b/Makefile index 9ba96a2f..41308a0c 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ BUNDLE_METADATA_OPTS ?= $(BUNDLE_CHANNELS) $(BUNDLE_DEFAULT_CHANNEL) # Image URL to use all building/pushing image targets IMG ?= controller:latest # Produce CRDs that work back to Kubernetes 1.11 (no version conversion) -CRD_OPTIONS ?= "crd:trivialVersions=true" +CRD_OPTIONS ?= "crd:trivialVersions=true,preserveUnknownFields=false" # Image URL for the operator final image OPERATOR_IMG ?= quay.io/m88i/nexus-operator:$(VERSION) diff --git a/bundle/manifests/apps.m88i.io_nexus.yaml b/bundle/manifests/apps.m88i.io_nexus.yaml index 3df4689b..175a26fc 100644 --- a/bundle/manifests/apps.m88i.io_nexus.yaml +++ b/bundle/manifests/apps.m88i.io_nexus.yaml @@ -2,6 +2,7 @@ apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: annotations: + cert-manager.io/inject-ca-from: nexus-operator-system/nexus-operator-serving-cert controller-gen.kubebuilder.io/version: v0.3.0 creationTimestamp: null name: nexus.apps.m88i.io @@ -27,12 +28,22 @@ spec: description: Internal Group Maven Public URL name: Maven Public URL type: string + conversion: + strategy: Webhook + webhookClientConfig: + caBundle: Cg== + service: + name: nexus-operator-webhook-service + namespace: nexus-operator-system + path: /convert + port: 443 group: apps.m88i.io names: kind: Nexus listKind: NexusList plural: nexus singular: nexus + preserveUnknownFields: false scope: Namespaced subresources: status: {} diff --git a/bundle/manifests/nexus-operator.clusterserviceversion.yaml b/bundle/manifests/nexus-operator.clusterserviceversion.yaml index c08bddbc..62140661 100644 --- a/bundle/manifests/nexus-operator.clusterserviceversion.yaml +++ b/bundle/manifests/nexus-operator.clusterserviceversion.yaml @@ -232,11 +232,24 @@ spec: - /manager image: quay.io/m88i/nexus-operator:0.5.0 name: manager + ports: + - containerPort: 9443 + name: webhook-server + protocol: TCP resources: requests: cpu: 100m memory: 20Mi + volumeMounts: + - mountPath: /tmp/k8s-webhook-server/serving-certs + name: cert + readOnly: true terminationGracePeriodSeconds: 10 + volumes: + - name: cert + secret: + defaultMode: 420 + secretName: webhook-server-cert permissions: - rules: - apiGroups: @@ -303,3 +316,38 @@ spec: name: m88i Labs replaces: nexus-operator.v0.4.0 version: 0.5.0 + webhookdefinitions: + - admissionReviewVersions: null + deploymentName: nexus-operator-controller-manager + failurePolicy: Fail + generateName: vnexus.kb.io + rules: + - apiGroups: + - apps.m88i.io.m88i.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - nexus + sideEffects: null + type: ValidatingAdmissionWebhook + webhookPath: /validate-apps-m88i-io-m88i-io-v1alpha1-nexus + - admissionReviewVersions: null + deploymentName: nexus-operator-controller-manager + failurePolicy: Fail + generateName: mnexus.kb.io + rules: + - apiGroups: + - apps.m88i.io.m88i.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - nexus + sideEffects: null + type: MutatingAdmissionWebhook + webhookPath: /mutate-apps-m88i-io-m88i-io-v1alpha1-nexus diff --git a/config/crd/bases/apps.m88i.io_nexus.yaml b/config/crd/bases/apps.m88i.io_nexus.yaml index 679410e9..91fca679 100644 --- a/config/crd/bases/apps.m88i.io_nexus.yaml +++ b/config/crd/bases/apps.m88i.io_nexus.yaml @@ -35,6 +35,7 @@ spec: listKind: NexusList plural: nexus singular: nexus + preserveUnknownFields: false scope: Namespaced subresources: status: {} diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index de2499ae..812c680a 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -8,12 +8,12 @@ resources: patchesStrategicMerge: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD -#- patches/webhook_in_nexus.yaml +- patches/webhook_in_nexus.yaml # +kubebuilder:scaffold:crdkustomizewebhookpatch # [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix. # patches here are for enabling the CA injection for each CRD -#- patches/cainjection_in_nexus.yaml +- patches/cainjection_in_nexus.yaml # +kubebuilder:scaffold:crdkustomizecainjectionpatch # the following config is for teaching kustomize how to do kustomization for CRDs. diff --git a/config/crd/patches/webhook_in_nexus.yaml b/config/crd/patches/webhook_in_nexus.yaml index ca842e70..7f1ce0ef 100644 --- a/config/crd/patches/webhook_in_nexus.yaml +++ b/config/crd/patches/webhook_in_nexus.yaml @@ -15,3 +15,4 @@ spec: namespace: system name: webhook-service path: /convert + port: 443 diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 2765f3c2..4e86e0aa 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -18,9 +18,9 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -#- ../webhook +- ../webhook # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. -#- ../certmanager +- ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. - ../prometheus @@ -32,39 +32,39 @@ patchesStrategicMerge: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -#- manager_webhook_patch.yaml +- manager_webhook_patch.yaml # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. # Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. # 'CERTMANAGER' needs to be enabled to use ca injection -#- webhookcainjection_patch.yaml +- webhookcainjection_patch.yaml # the following config is for teaching kustomize how to do var substitution -#vars: +vars: # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. -#- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR -# objref: -# kind: Certificate -# group: cert-manager.io -# version: v1alpha2 -# name: serving-cert # this name should match the one in certificate.yaml -# fieldref: -# fieldpath: metadata.namespace -#- name: CERTIFICATE_NAME -# objref: -# kind: Certificate -# group: cert-manager.io -# version: v1alpha2 -# name: serving-cert # this name should match the one in certificate.yaml -#- name: SERVICE_NAMESPACE # namespace of the service -# objref: -# kind: Service -# version: v1 -# name: webhook-service -# fieldref: -# fieldpath: metadata.namespace -#- name: SERVICE_NAME -# objref: -# kind: Service -# version: v1 -# name: webhook-service +- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR + objref: + kind: Certificate + group: cert-manager.io + version: v1alpha2 + name: serving-cert # this name should match the one in certificate.yaml + fieldref: + fieldpath: metadata.namespace +- name: CERTIFICATE_NAME + objref: + kind: Certificate + group: cert-manager.io + version: v1alpha2 + name: serving-cert # this name should match the one in certificate.yaml +- name: SERVICE_NAMESPACE # namespace of the service + objref: + kind: Service + version: v1 + name: webhook-service + fieldref: + fieldpath: metadata.namespace +- name: SERVICE_NAME + objref: + kind: Service + version: v1 + name: webhook-service diff --git a/nexus-operator.yaml b/nexus-operator.yaml index dbce795c..dcfffb99 100644 --- a/nexus-operator.yaml +++ b/nexus-operator.yaml @@ -9,6 +9,7 @@ apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: annotations: + cert-manager.io/inject-ca-from: nexus-operator-system/nexus-operator-serving-cert controller-gen.kubebuilder.io/version: v0.3.0 creationTimestamp: null name: nexus.apps.m88i.io @@ -34,12 +35,22 @@ spec: description: Internal Group Maven Public URL name: Maven Public URL type: string + conversion: + strategy: Webhook + webhookClientConfig: + caBundle: Cg== + service: + name: nexus-operator-webhook-service + namespace: nexus-operator-system + path: /convert + port: 443 group: apps.m88i.io names: kind: Nexus listKind: NexusList plural: nexus singular: nexus + preserveUnknownFields: false scope: Namespaced subresources: status: {} @@ -429,6 +440,33 @@ status: conditions: [] storedVersions: [] --- +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: MutatingWebhookConfiguration +metadata: + annotations: + cert-manager.io/inject-ca-from: nexus-operator-system/nexus-operator-serving-cert + creationTimestamp: null + name: nexus-operator-mutating-webhook-configuration +webhooks: +- clientConfig: + caBundle: Cg== + service: + name: nexus-operator-webhook-service + namespace: nexus-operator-system + path: /mutate-apps-m88i-io-m88i-io-v1alpha1-nexus + failurePolicy: Fail + name: mnexus.kb.io + rules: + - apiGroups: + - apps.m88i.io.m88i.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - nexus +--- apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: @@ -665,6 +703,18 @@ spec: selector: control-plane: controller-manager --- +apiVersion: v1 +kind: Service +metadata: + name: nexus-operator-webhook-service + namespace: nexus-operator-system +spec: + ports: + - port: 443 + targetPort: 9443 + selector: + control-plane: controller-manager +--- apiVersion: apps/v1 kind: Deployment metadata: @@ -700,11 +750,46 @@ spec: - /manager image: quay.io/m88i/nexus-operator:0.5.0 name: manager + ports: + - containerPort: 9443 + name: webhook-server + protocol: TCP resources: requests: cpu: 100m memory: 20Mi + volumeMounts: + - mountPath: /tmp/k8s-webhook-server/serving-certs + name: cert + readOnly: true terminationGracePeriodSeconds: 10 + volumes: + - name: cert + secret: + defaultMode: 420 + secretName: webhook-server-cert +--- +apiVersion: cert-manager.io/v1alpha2 +kind: Certificate +metadata: + name: nexus-operator-serving-cert + namespace: nexus-operator-system +spec: + dnsNames: + - nexus-operator-webhook-service.nexus-operator-system.svc + - nexus-operator-webhook-service.nexus-operator-system.svc.cluster.local + issuerRef: + kind: Issuer + name: nexus-operator-selfsigned-issuer + secretName: webhook-server-cert +--- +apiVersion: cert-manager.io/v1alpha2 +kind: Issuer +metadata: + name: nexus-operator-selfsigned-issuer + namespace: nexus-operator-system +spec: + selfSigned: {} --- apiVersion: monitoring.coreos.com/v1 kind: ServiceMonitor @@ -720,3 +805,30 @@ spec: selector: matchLabels: control-plane: controller-manager +--- +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: ValidatingWebhookConfiguration +metadata: + annotations: + cert-manager.io/inject-ca-from: nexus-operator-system/nexus-operator-serving-cert + creationTimestamp: null + name: nexus-operator-validating-webhook-configuration +webhooks: +- clientConfig: + caBundle: Cg== + service: + name: nexus-operator-webhook-service + namespace: nexus-operator-system + path: /validate-apps-m88i-io-m88i-io-v1alpha1-nexus + failurePolicy: Fail + name: vnexus.kb.io + rules: + - apiGroups: + - apps.m88i.io.m88i.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - nexus From 9a86fc7c269de52286c4d048c1dae9bbfd43f8f7 Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Mon, 1 Feb 2021 18:44:13 -0300 Subject: [PATCH 05/10] Fix webhook group --- api/v1alpha1/nexus_webhook.go | 6 ++-- api/v1alpha1/validation.go | 23 ++++++++++++++- api/v1alpha1/validation_test.go | 29 ++++++++++++++++++- ...s-operator-webhook-service_v1_service.yaml | 13 +++++++++ .../nexus-operator.clusterserviceversion.yaml | 22 +++++++++----- config/manager/kustomization.yaml | 4 +-- config/webhook/manifests.yaml | 8 ++--- nexus-operator.yaml | 8 ++--- pkg/framework/tags.go | 5 ++++ 9 files changed, 94 insertions(+), 24 deletions(-) create mode 100644 bundle/manifests/nexus-operator-webhook-service_v1_service.yaml diff --git a/api/v1alpha1/nexus_webhook.go b/api/v1alpha1/nexus_webhook.go index 2316b8f2..9c5e10a4 100644 --- a/api/v1alpha1/nexus_webhook.go +++ b/api/v1alpha1/nexus_webhook.go @@ -33,9 +33,7 @@ func (n *Nexus) SetupWebhookWithManager(mgr ctrl.Manager) error { Complete() } -// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! - -// +kubebuilder:webhook:path=/mutate-apps-m88i-io-m88i-io-v1alpha1-nexus,mutating=true,failurePolicy=fail,groups=apps.m88i.io.m88i.io,resources=nexus,verbs=create;update,versions=v1alpha1,name=mnexus.kb.io +// +kubebuilder:webhook:path=/mutate-apps-m88i-io-v1alpha1-nexus,mutating=true,failurePolicy=fail,groups=apps.m88i.io,resources=nexus,verbs=create;update,versions=v1alpha1,name=mnexus.kb.io var _ webhook.Defaulter = &Nexus{} @@ -49,7 +47,7 @@ func (n *Nexus) Default() { } // if we ever need validation upon delete requests change verbs to "verbs=create;update;delete". -// +kubebuilder:webhook:verbs=create;update,path=/validate-apps-m88i-io-m88i-io-v1alpha1-nexus,mutating=false,failurePolicy=fail,groups=apps.m88i.io.m88i.io,resources=nexus,versions=v1alpha1,name=vnexus.kb.io +// +kubebuilder:webhook:verbs=create;update,path=/validate-apps-m88i-io-v1alpha1-nexus,mutating=false,failurePolicy=fail,groups=apps.m88i.io,resources=nexus,versions=v1alpha1,name=vnexus.kb.io var _ webhook.Validator = &Nexus{} diff --git a/api/v1alpha1/validation.go b/api/v1alpha1/validation.go index e7b5acb5..b44bc179 100644 --- a/api/v1alpha1/validation.go +++ b/api/v1alpha1/validation.go @@ -17,6 +17,8 @@ package v1alpha1 import ( "fmt" + "k8s.io/apimachinery/pkg/api/resource" + "github.com/m88i/nexus-operator/pkg/cluster/discovery" "github.com/m88i/nexus-operator/pkg/logger" ) @@ -53,7 +55,17 @@ func newValidator(nexus *Nexus) *validator { // validate returns an error if the Nexus CR is invalid func (v *validator) validate() error { - return v.validateNetworking() + validatingFuncs := []func() error{ + v.validateNetworking, + v.validatePersistence, + } + + for _, fn := range validatingFuncs { + if err := fn(); err != nil { + return err + } + } + return nil } func (v *validator) validateNetworking() error { @@ -94,3 +106,12 @@ func (v *validator) validateNetworking() error { return nil } + +func (v *validator) validatePersistence() error { + vSize := v.nexus.Spec.Persistence.VolumeSize + if _, err := resource.ParseQuantity(vSize); err != nil { + v.log.Warn("'spec.persistence.volumeSize invalid", "volumeSize", vSize) + return fmt.Errorf("'spec.persistence.volumeSize (%s) invalid: %v", vSize, err) + } + return nil +} diff --git a/api/v1alpha1/validation_test.go b/api/v1alpha1/validation_test.go index 3a3f8b66..d807e142 100644 --- a/api/v1alpha1/validation_test.go +++ b/api/v1alpha1/validation_test.go @@ -175,7 +175,34 @@ func TestValidator_validate_Networking(t *testing.T) { for _, tt := range tests { discovery.SetClient(tt.disc) - err := newValidator(tt.input).validate() + err := newValidator(tt.input).validateNetworking() + if (err != nil) != tt.wantErr { + t.Errorf("%s\nnexus: %#v\nwantErr: %v\terr: %#v\n", tt.name, tt.input, tt.wantErr, err) + } + } +} + +func TestValidator_validate_Persistence(t *testing.T) { + tests := []struct { + name string + input *Nexus + wantErr bool + }{ + { + "Invalid volume size", + &Nexus{Spec: NexusSpec{Persistence: NexusPersistence{VolumeSize: "not-a-quantity"}}}, + true, + }, + { + "Valid volume size", + &Nexus{Spec: NexusSpec{Persistence: NexusPersistence{VolumeSize: "10Gi"}}}, + false, + }, + } + + discovery.SetClient(discovery.NewFakeDiscBuilder().Build()) + for _, tt := range tests { + err := newValidator(tt.input).validatePersistence() if (err != nil) != tt.wantErr { t.Errorf("%s\nnexus: %#v\nwantErr: %v\terr: %#v\n", tt.name, tt.input, tt.wantErr, err) } diff --git a/bundle/manifests/nexus-operator-webhook-service_v1_service.yaml b/bundle/manifests/nexus-operator-webhook-service_v1_service.yaml new file mode 100644 index 00000000..19c9fec4 --- /dev/null +++ b/bundle/manifests/nexus-operator-webhook-service_v1_service.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Service +metadata: + creationTimestamp: null + name: nexus-operator-webhook-service +spec: + ports: + - port: 443 + targetPort: 9443 + selector: + control-plane: controller-manager +status: + loadBalancer: {} diff --git a/bundle/manifests/nexus-operator.clusterserviceversion.yaml b/bundle/manifests/nexus-operator.clusterserviceversion.yaml index 62140661..ed82b616 100644 --- a/bundle/manifests/nexus-operator.clusterserviceversion.yaml +++ b/bundle/manifests/nexus-operator.clusterserviceversion.yaml @@ -317,13 +317,15 @@ spec: replaces: nexus-operator.v0.4.0 version: 0.5.0 webhookdefinitions: - - admissionReviewVersions: null + - admissionReviewVersions: + - v1beta1 + containerPort: 443 deploymentName: nexus-operator-controller-manager failurePolicy: Fail generateName: vnexus.kb.io rules: - apiGroups: - - apps.m88i.io.m88i.io + - apps.m88i.io apiVersions: - v1alpha1 operations: @@ -331,16 +333,19 @@ spec: - UPDATE resources: - nexus - sideEffects: null + sideEffects: None + targetPort: 9443 type: ValidatingAdmissionWebhook - webhookPath: /validate-apps-m88i-io-m88i-io-v1alpha1-nexus - - admissionReviewVersions: null + webhookPath: /validate-apps-m88i-io-v1alpha1-nexus + - admissionReviewVersions: + - v1beta1 + containerPort: 443 deploymentName: nexus-operator-controller-manager failurePolicy: Fail generateName: mnexus.kb.io rules: - apiGroups: - - apps.m88i.io.m88i.io + - apps.m88i.io apiVersions: - v1alpha1 operations: @@ -348,6 +353,7 @@ spec: - UPDATE resources: - nexus - sideEffects: null + sideEffects: None + targetPort: 9443 type: MutatingAdmissionWebhook - webhookPath: /mutate-apps-m88i-io-m88i-io-v1alpha1-nexus + webhookPath: /mutate-apps-m88i-io-v1alpha1-nexus diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 6a0cd6c1..5cf98bf8 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -4,5 +4,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization images: - name: controller - newName: quay.io/m88i/nexus-operator - newTag: 0.5.0 + newName: controller + newTag: v1 diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 680da4cf..4a0874a5 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -11,12 +11,12 @@ webhooks: service: name: webhook-service namespace: system - path: /mutate-apps-m88i-io-m88i-io-v1alpha1-nexus + path: /mutate-apps-m88i-io-v1alpha1-nexus failurePolicy: Fail name: mnexus.kb.io rules: - apiGroups: - - apps.m88i.io.m88i.io + - apps.m88i.io apiVersions: - v1alpha1 operations: @@ -37,12 +37,12 @@ webhooks: service: name: webhook-service namespace: system - path: /validate-apps-m88i-io-m88i-io-v1alpha1-nexus + path: /validate-apps-m88i-io-v1alpha1-nexus failurePolicy: Fail name: vnexus.kb.io rules: - apiGroups: - - apps.m88i.io.m88i.io + - apps.m88i.io apiVersions: - v1alpha1 operations: diff --git a/nexus-operator.yaml b/nexus-operator.yaml index dcfffb99..a44a6bda 100644 --- a/nexus-operator.yaml +++ b/nexus-operator.yaml @@ -453,12 +453,12 @@ webhooks: service: name: nexus-operator-webhook-service namespace: nexus-operator-system - path: /mutate-apps-m88i-io-m88i-io-v1alpha1-nexus + path: /mutate-apps-m88i-io-v1alpha1-nexus failurePolicy: Fail name: mnexus.kb.io rules: - apiGroups: - - apps.m88i.io.m88i.io + - apps.m88i.io apiVersions: - v1alpha1 operations: @@ -819,12 +819,12 @@ webhooks: service: name: nexus-operator-webhook-service namespace: nexus-operator-system - path: /validate-apps-m88i-io-m88i-io-v1alpha1-nexus + path: /validate-apps-m88i-io-v1alpha1-nexus failurePolicy: Fail name: vnexus.kb.io rules: - apiGroups: - - apps.m88i.io.m88i.io + - apps.m88i.io apiVersions: - v1alpha1 operations: diff --git a/pkg/framework/tags.go b/pkg/framework/tags.go index e0e495d9..32fba55f 100644 --- a/pkg/framework/tags.go +++ b/pkg/framework/tags.go @@ -41,6 +41,11 @@ var ( latestMicros = make(map[int]string) ) +func init() { + // fetch the tags once during startup to improve responsiveness of the mutation webhook's first run + fetchUpdates() +} + // HigherVersion checks if thisTag is of a higher version than otherTag func HigherVersion(thisTag, otherTag string) (bool, error) { thisMinor, err := GetMinor(thisTag) From accc48ce2f735a1778bb724b9147b8450790c7d2 Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Mon, 1 Feb 2021 20:40:35 -0300 Subject: [PATCH 06/10] Inform reader no changes or validation takes place in e2e tests Signed-off-by: Lucas Caparelli --- CONTRIBUTING.md | 164 ++++-------------------------- config/manager/kustomization.yaml | 2 +- 2 files changed, 23 insertions(+), 143 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a0a9199e..c935fbce 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,16 +10,27 @@ We're happy to answer! Either [open an issue](https://github.com/m88i/nexus-oper ## Are you willing to send a PR? -Before sending a PR, consider opening an issue first. This way we can discuss your approach and the motivations behind it while also taking into account other development efforts. +Before sending a PR, consider opening an issue first. This way we can discuss your approach and the motivations behind +it while also taking into account other development efforts. Regarding your local development environment: -1. We use [golint-ci](https://golangci-lint.run/) to check the code. Consider [integrating it in your favorite IDE](https://golangci-lint.run/usage/integrations/) to avoid failing in the CI -2. **Always** run `make test` before sending a PR to make sure the license headers and the manifests are updated (and of course the unit tests are passing) -3. Consider adding a new [end-to-end](https://sdk.operatorframework.io/docs/golang/e2e-tests/) test case covering your scenario and make sure to run `make test-e2e` before sending the PR -4. Make sure to always keep your version of `Go` and the `operator-sdk` on par with the project. The current version information can be found at [the go.mod file](go.mod) +1. We use [golint-ci](https://golangci-lint.run/) to check the code. + Consider [integrating it in your favorite IDE](https://golangci-lint.run/usage/integrations/) to avoid failing in the + CI +2. **Always** run `make test` before sending a PR to make sure the license headers and the manifests are updated (and of + course the unit tests are passing) +3. Consider adding a new [end-to-end](https://sdk.operatorframework.io/docs/building-operators/golang/testing/) test + case covering your scenario in `controllers/nexus_controller_test.go` and make sure to run `make test` before sending + the PR +4. Make sure to always keep your version of `go` and the `operator-sdk` on par with the project. -To run all unit tests, build the image, run the E2E tests with that image and push, all in one go, you may run `make pr-prep`. If any tests fail or if the build fails, the process will be terminated so that you make the necessary adjustments. If they are all successful, you'll be prompted to push your commited changes. + - go 1.15 + - operator-sdk v1.2.0 + +To run all tests and push, all in one go, you may run `make pr-prep`. If any tests fail or if the build fails, the +process will be terminated so that you make the necessary adjustments. If they are all successful, you'll be prompted to +push your committed changes. ```shell $ make pr-prep @@ -46,140 +57,9 @@ Pushing to origin/pr-prep ## E2E Testing -If you added a new functionality and are willing to add some end-to-end (E2E) testing of your own, please add a test case to `test/e2e/nexus_test.go`. - -The test case structure allows you to name your test appropriately (try naming it in a way it's clear what it's testing), provide a Nexus CR that the Operator will use to generate the other resources, provide additional checks your feature may require and provide a custom cleanup function if necessary. - -Then each test case is submitted to a series of checks which should make sure everything on the cluster is as it should, based on the Nexus CR that has been defined. - -Let's take our smoke test as an example to go over the test cases structure: - -```go -testCases := []struct { - name string (1) - input *v1alpha1.Nexus (2) - cleanup func() error (3) - additionalChecks []func(nexus *v1alpha1.Nexus) error (4) -}{ - { - name: "Smoke test: no persistence, nodeport exposure", (1) - input: &v1alpha1.Nexus{ (2) - ObjectMeta: metav1.ObjectMeta{ - Name: nexusName, - Namespace: namespace, - }, - Spec: defaultNexusSpec, (5) - }, - cleanup: tester.defaultCleanup, (3) - additionalChecks: nil, (4) - }, -``` - -> (1): the test case's name. In this scenario we're testing a deployment with all default values, no persistence and exposed via Node Port.
-> (2): the Nexus CR which the Operator will use to orchestrate and maintain your Nexus3 deployment
-> (3): a cleanup function which should be ran after the test has been completed
-> (4): additional checks your test case may need
-> (5): the base, default Nexus CR specification which should be used for testing. Modify this to test your own features
- -**Important**: although the operator will set the defaults on the Nexus CR you provide it with, the tests will use your original CR for comparison, so be sure to make a completely valid Nexus CR for your test case as it *will not* be modified to insert default values. - -### Custom Nexus CR - -If your test requires modifications to the default Nexus CR, you can do so directly and concisely when defining the test case by making use of anonymous functions. - -For example: - -```go -{ - name: "Networking: ingress with no TLS", - input: &v1alpha1.Nexus{ - ObjectMeta: metav1.ObjectMeta{ - Name: nexusName, - Namespace: namespace, - }, - Spec: func() v1alpha1.NexusSpec { - spec := *defaultNexusSpec.DeepCopy() - spec.Networking = v1alpha1.NexusNetworking{Expose: true, ExposeAs: v1alpha1.IngressExposeType, Host: "test-example.com"} - return spec - }(), - }, - cleanup: tester.defaultCleanup, - additionalChecks: nil, -}, -``` - -When defining the Nexus's specification in this case we're actually calling an anonymous function that acquires the default spec, modifies the required fields and then returns that spec, thus making the necessary changes for the test. - -### Custom Cleanup functions - -Our test cases make use of [functions first-class citizenship in Go](https://golang.org/doc/codewalk/functions/) by declaring the cleanup function as a field from the test case. This way it's possible to specify our own custom cleanup function for a test. - -In previous examples, `tester.defaultCleanup` was used, which simply deletes all Nexus CRs in the namespace, but you may want to do some additional computation when cleaning up, such as counting to 5 (intentionally useless to promote simplicity in this example): - -```go -{ - name: "Test Example: this counts to 5 during cleanup and uses the default cleanup once done", - input: &v1alpha1.Nexus{ - ObjectMeta: metav1.ObjectMeta{ - Name: nexusName, - Namespace: namespace, - }, - Spec: defaultNexusSpec, - }, - cleanup: func() error { - for i := 0; i < 5; i++ { - tester.t.Logf("Count: %d", i) - } - return tester.defaultCleanup() - }, - additionalChecks: nil, -}, -``` - -It's possible, of course, to not use the default cleanup function at all, but be sure to actually delete the resources you created if they conflict with other test cases (the framework itself will delete the whole namespace once the tests are done): - -```go -{ - name: "Test Example: this only counts to 5 during cleanup and does not delete anything", - input: &v1alpha1.Nexus{ - ObjectMeta: metav1.ObjectMeta{ - Name: nexusName, - Namespace: namespace, - }, - Spec: defaultNexusSpec, - }, - cleanup: func() error { - for i := 0; i < 5; i++ { - tester.t.Logf("Count: %d", i) - } - return nil - }, - additionalChecks: nil, -}, -``` +If you added a new functionality and are willing to add some end-to-end (E2E) testing of your own, please add a test +case to `controllers/nexus_controller_test.go`. -### Running additional checks - -If your testing needs to check something that isn't already checked by default you may add functions to perform these checks as the function that is responsible for running the default checks will receive them as [variadic arguments](https://gobyexample.com/variadic-functions). - -In another useless yet simple example, let's also make sure that 5 is greater than 4 when performing our checks: - -```go -{ - name: "Test Example: this will also check if 5 > 4", - input: &v1alpha1.Nexus{ - ObjectMeta: metav1.ObjectMeta{ - Name: nexusName, - Namespace: namespace, - }, - Spec: defaultNexusSpec, - }, - cleanup: tester.defaultCleanup, - additionalChecks: []func(nexus *v1alpha1.Nexus)error{ - func(nexus *v1alpha1.Nexus) error { - assert.Greater(tester.t, 5, 4) - return nil - }, - }, -}, -``` \ No newline at end of file +All mutating (including default values) and validating logic lives in our admission webhooks, which are not in place +when the e2e suite runs. Because of that, no changes or checks are performed against Nexus CRs being reconciled. Be sure +to always write tests which use valid Nexus resources. \ No newline at end of file diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 5cf98bf8..ad13e96b 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -5,4 +5,4 @@ kind: Kustomization images: - name: controller newName: controller - newTag: v1 + newTag: latest From 28bd7f3a796d53f931315886754d28bf38aa080d Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Tue, 2 Feb 2021 00:37:33 -0300 Subject: [PATCH 07/10] Create auto-generation for webhookless installer manifest Signed-off-by: Lucas Caparelli --- Makefile | 11 +- webhookless-nexus-operator.yaml | 740 ++++++++++++++++++++++++++++++++ 2 files changed, 749 insertions(+), 2 deletions(-) create mode 100644 webhookless-nexus-operator.yaml diff --git a/Makefile b/Makefile index 41308a0c..0241f47c 100644 --- a/Makefile +++ b/Makefile @@ -40,14 +40,14 @@ test: generate-installer fmt vet bundle test-only # just test without generating anything, use wisely test-only: - mkdir -p ${ENVTEST_ASSETS_DIR} + mkdir -p ${ENVTEST_ASSETS_DIR} test -f ${ENVTEST_ASSETS_DIR}/setup-envtest.sh || curl -sSLo ${ENVTEST_ASSETS_DIR}/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/master/hack/setup-envtest.sh sed -i "s,#\!.*,#\!\/bin\/bash,g" ${ENVTEST_ASSETS_DIR}/setup-envtest.sh sed -i "/pipefail/d" ${ENVTEST_ASSETS_DIR}/setup-envtest.sh source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; ENVTEST_K8S_VERSION=$(K8S_VERSION) fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out -generate-installer: generate manifests kustomize +generate-installer: generate manifests kustomize generate-webhookless-installer cd config/manager && $(KUSTOMIZE) edit set image controller=$(OPERATOR_IMG) $(KUSTOMIZE) build config/default > nexus-operator.yaml @@ -161,3 +161,10 @@ create_namespace=true run_with_image=true pr-prep: CREATE_NAMESPACE=$(create_namespace) RUN_WITH_IMAGE=$(run_with_image) ./hack/pr-prep.sh + +generate-webhookless-installer: + # first, let's filter out all manifests we don't care about + # then delete the volumes which would contain the certs + # then finally insert the env var which disables webhooks + # TODO : find a way to make this more readable + kustomize build config/default/ | yq -Y 'select(.kind != "ValidatingWebhookConfiguration" and .kind != "Issuer" and .kind != "Certificate" and .kind != "MutatingWebhookConfiguration" and .metadata.name != "nexus-operator-webhook-service")' | yq -Y 'del(.. | .volumes?, .volumeMounts?)' | yq -Y 'if .kind=="Deployment" then .spec.template.spec.containers[1].env[0]={"name":"USE_WEBHOOKS", "value":"FALSE"} else . end' > webhookless-nexus-operator.yaml diff --git a/webhookless-nexus-operator.yaml b/webhookless-nexus-operator.yaml new file mode 100644 index 00000000..4d0506c2 --- /dev/null +++ b/webhookless-nexus-operator.yaml @@ -0,0 +1,740 @@ +apiVersion: v1 +kind: Namespace +metadata: + labels: + control-plane: controller-manager + name: nexus-operator-system +--- +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: nexus-operator-system/nexus-operator-serving-cert + controller-gen.kubebuilder.io/version: v0.3.0 + creationTimestamp: null + name: nexus.apps.m88i.io +spec: + additionalPrinterColumns: + - JSONPath: .spec.networking.exposeAs + description: Type of networking access + name: Expose As + type: string + - JSONPath: .spec.automaticUpdate.disabled + description: Flag that indicates if automatic updates are disabled or not + name: Update Disabled + type: boolean + - JSONPath: .status.nexusStatus + description: Instance Status + name: Status + type: string + - JSONPath: .status.reason + description: Status reason + name: Reason + type: string + - JSONPath: .status.serverOperationsStatus.mavenPublicURL + description: Internal Group Maven Public URL + name: Maven Public URL + type: string + conversion: + strategy: Webhook + webhookClientConfig: + caBundle: Cg== + service: + name: nexus-operator-webhook-service + namespace: nexus-operator-system + path: /convert + port: 443 + group: apps.m88i.io + names: + kind: Nexus + listKind: NexusList + plural: nexus + singular: nexus + preserveUnknownFields: false + scope: Namespaced + subresources: + status: { } + validation: + openAPIV3Schema: + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: NexusSpec defines the desired state of Nexus + properties: + automaticUpdate: + description: Automatic updates configuration + properties: + disabled: + description: Whether or not the Operator should perform automatic + updates. Defaults to `false` (auto updates are enabled). Is set + to `false` if `spec.image` is not empty and is different from + the default community image. + type: boolean + minorVersion: + description: The Nexus image minor version the deployment should + stay in. If left blank and automatic updates are enabled the latest + minor is set. + minimum: 0 + type: integer + type: object + generateRandomAdminPassword: + description: 'GenerateRandomAdminPassword enables the random password + generation. Defaults to `false`: the default password for a newly + created instance is ''admin123'', which should be changed in the first + login. If set to `true`, you must use the automatically generated + ''admin'' password, stored in the container''s file system at `/nexus-data/admin.password`. + The operator uses the default credentials to create a user for itself + to create default repositories. If set to `true`, the repositories + won''t be created since the operator won''t fetch for the random password.' + type: boolean + image: + description: 'Full image tag name for this specific deployment. Will + be ignored if `spec.useRedHatImage` is set to `true`. Default: docker.io/sonatype/nexus3:latest' + type: string + imagePullPolicy: + description: 'The image pull policy for the Nexus image. If left blank + behavior will be determined by the image tag (`Always` if "latest" + and `IfNotPresent` otherwise). Possible values: `Always`, `IfNotPresent` + or `Never`.' + enum: + - Always + - IfNotPresent + - Never + type: string + livenessProbe: + description: LivenessProbe describes how the Nexus container liveness + probe should work + properties: + failureThreshold: + description: Minimum consecutive failures for the probe to be considered + failed after having succeeded. Defaults to 3. Minimum value is + 1. + format: int32 + minimum: 1 + type: integer + initialDelaySeconds: + description: Number of seconds after the container has started before + probes are initiated. Defaults to 240 seconds. Minimum value is + 0. + format: int32 + minimum: 0 + type: integer + periodSeconds: + description: How often (in seconds) to perform the probe. Defaults + to 10 seconds. Minimum value is 1. + format: int32 + minimum: 1 + type: integer + successThreshold: + description: Minimum consecutive successes for the probe to be considered + successful after having failed. Defaults to 1. Must be 1 for liveness + and startup. Minimum value is 1. + format: int32 + minimum: 1 + type: integer + timeoutSeconds: + description: Number of seconds after which the probe times out. + Defaults to 15 seconds. Minimum value is 1. + format: int32 + minimum: 1 + type: integer + type: object + networking: + description: Networking definition + properties: + expose: + description: Set to `true` to expose the Nexus application. Defaults + to `false`. + type: boolean + exposeAs: + description: 'Type of networking exposure: NodePort, Route or Ingress. + Defaults to Route on OpenShift and Ingress on Kubernetes. Routes + are only available on Openshift and Ingresses are only available + on Kubernetes.' + enum: + - NodePort + - Route + - Ingress + type: string + host: + description: Host where the Nexus service is exposed. This attribute + is required if the service is exposed via Ingress. + type: string + nodePort: + description: NodePort defined in the exposed service. Required if + exposed via NodePort. + format: int32 + type: integer + tls: + description: TLS/SSL-related configuration + properties: + mandatory: + description: When exposing via Route, set to `true` to only + allow encrypted traffic using TLS (disables HTTP in favor + of HTTPS). Defaults to `false`. + type: boolean + secretName: + description: When exposing via Ingress, inform the name of the + TLS secret containing certificate and private key for TLS + encryption. It must be present in the same namespace as the + Operator. + type: string + type: object + type: object + persistence: + description: Persistence definition + properties: + persistent: + description: Flag to indicate if this instance will be persistent + or not + type: boolean + storageClass: + description: StorageClass used by the managed PVC. + type: string + volumeSize: + description: 'If persistent, the size of the Volume. Defaults: 10Gi' + type: string + required: + - persistent + type: object + readinessProbe: + description: ReadinessProbe describes how the Nexus container readiness + probe should work + properties: + failureThreshold: + description: Minimum consecutive failures for the probe to be considered + failed after having succeeded. Defaults to 3. Minimum value is + 1. + format: int32 + minimum: 1 + type: integer + initialDelaySeconds: + description: Number of seconds after the container has started before + probes are initiated. Defaults to 240 seconds. Minimum value is + 0. + format: int32 + minimum: 0 + type: integer + periodSeconds: + description: How often (in seconds) to perform the probe. Defaults + to 10 seconds. Minimum value is 1. + format: int32 + minimum: 1 + type: integer + successThreshold: + description: Minimum consecutive successes for the probe to be considered + successful after having failed. Defaults to 1. Must be 1 for liveness + and startup. Minimum value is 1. + format: int32 + minimum: 1 + type: integer + timeoutSeconds: + description: Number of seconds after which the probe times out. + Defaults to 15 seconds. Minimum value is 1. + format: int32 + minimum: 1 + type: integer + type: object + replicas: + description: Number of pod replicas desired. Defaults to 0. + format: int32 + maximum: 100 + minimum: 0 + type: integer + resources: + description: Defined Resources for the Nexus instance + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute resources + allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute resources + required. If Requests is omitted for a container, it defaults + to Limits if that is explicitly specified, otherwise to an implementation-defined + value. More info: https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/' + type: object + type: object + serverOperations: + description: ServerOperations describes the options for the operations + performed on the deployed server instance + properties: + disableOperatorUserCreation: + description: DisableOperatorUserCreation disables the auto-creation + of the `nexus-operator` user on the deployed server. This user + performs all the operations on the server (such as creating the + community repos). If disabled, the Operator will use the default + `admin` user. Defaults to `false` (always create the user). Setting + this to `true` is not recommended as it grants the Operator more + privileges than it needs and it would not be possible to tell + apart operations performed by the `admin` and the Operator. + type: boolean + disableRepositoryCreation: + description: DisableRepositoryCreation disables the auto-creation + of Apache, JBoss and Red Hat repositories and their addition to + the Maven Public group in this Nexus instance. Defaults to `false` + (always try to create the repos). Set this to `true` to not create + them. Only works if `spec.generateRandomAdminPassword` is `false`. + type: boolean + type: object + serviceAccountName: + description: ServiceAccountName is the name of the ServiceAccount used + to run the Pods. If left blank, a default ServiceAccount is created + with the same name as the Nexus CR (`metadata.name`). + type: string + useRedHatImage: + description: If you have access to Red Hat Container Catalog, set this + to `true` to use the certified image provided by Sonatype Defaults + to `false` + type: boolean + required: + - persistence + - replicas + - useRedHatImage + type: object + status: + description: NexusStatus defines the observed state of Nexus + properties: + deploymentStatus: + description: Condition status for the Nexus deployment + properties: + availableReplicas: + description: Total number of available pods (ready for at least + minReadySeconds) targeted by this deployment. + format: int32 + type: integer + collisionCount: + description: Count of hash collisions for the Deployment. The Deployment + controller uses this field as a collision avoidance mechanism + when it needs to create the name for the newest ReplicaSet. + format: int32 + type: integer + conditions: + description: Represents the latest available observations of a deployment's + current state. + items: + description: DeploymentCondition describes the state of a deployment + at a certain point. + properties: + lastTransitionTime: + description: Last time the condition transitioned from one + status to another. + format: date-time + type: string + lastUpdateTime: + description: The last time this condition was updated. + format: date-time + type: string + message: + description: A human readable message indicating details about + the transition. + type: string + reason: + description: The reason for the condition's last transition. + type: string + status: + description: Status of the condition, one of True, False, + Unknown. + type: string + type: + description: Type of deployment condition. + type: string + required: + - status + - type + type: object + type: array + observedGeneration: + description: The generation observed by the deployment controller. + format: int64 + type: integer + readyReplicas: + description: Total number of ready pods targeted by this deployment. + format: int32 + type: integer + replicas: + description: Total number of non-terminated pods targeted by this + deployment (their labels match the selector). + format: int32 + type: integer + unavailableReplicas: + description: Total number of unavailable pods targeted by this deployment. + This is the total number of pods that are still required for the + deployment to have 100% available capacity. They may either be + pods that are running but not yet available or pods that still + have not been created. + format: int32 + type: integer + updatedReplicas: + description: Total number of non-terminated pods targeted by this + deployment that have the desired template spec. + format: int32 + type: integer + type: object + nexusRoute: + description: Route for external service access + type: string + nexusStatus: + description: Will be "OK" when this Nexus instance is up + type: string + reason: + description: Gives more information about a failure status + type: string + serverOperationsStatus: + description: ServerOperationsStatus describes the general status for + the operations performed in the Nexus server instance + properties: + communityRepositoriesCreated: + type: boolean + mavenCentralUpdated: + type: boolean + mavenPublicURL: + type: string + operatorUserCreated: + type: boolean + reason: + type: string + serverReady: + type: boolean + type: object + updateConditions: + description: Conditions reached during an update + items: + type: string + type: array + x-kubernetes-list-type: atomic + type: object + type: object + version: v1alpha1 + versions: + - name: v1alpha1 + served: true + storage: true +status: + acceptedNames: + kind: "" + plural: "" + conditions: [ ] + storedVersions: [ ] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: nexus-operator-leader-election-role + namespace: nexus-operator-system +rules: + - apiGroups: + - "" + resources: + - configmaps + verbs: + - get + - list + - watch + - create + - update + - patch + - delete + - apiGroups: + - "" + resources: + - configmaps/status + verbs: + - get + - update + - patch + - apiGroups: + - "" + resources: + - events + verbs: + - create + - patch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + creationTimestamp: null + name: nexus-operator-manager-role +rules: + - apiGroups: + - apps + resources: + - deployments + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - apps + resources: + - deployments/finalizers + verbs: + - update + - apiGroups: + - apps + resources: + - replicasets + verbs: + - get + - apiGroups: + - apps.m88i.io + resources: + - nexus + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - apps.m88i.io + resources: + - nexus/finalizers + verbs: + - get + - patch + - update + - apiGroups: + - apps.m88i.io + resources: + - nexus/status + verbs: + - get + - patch + - update + - apiGroups: + - "" + resources: + - configmaps + verbs: + - create + - get + - apiGroups: + - "" + resources: + - events + - persistentvolumeclaims + - secrets + - serviceaccounts + - services + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - "" + resources: + - pods + verbs: + - get + - apiGroups: + - monitoring.coreos.com + resources: + - servicemonitors + verbs: + - create + - get + - apiGroups: + - networking.k8s.io + resources: + - ingresses + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - route.openshift.io + resources: + - routes + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: nexus-operator-proxy-role +rules: + - apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create + - apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create +--- +apiVersion: rbac.authorization.k8s.io/v1beta1 +kind: ClusterRole +metadata: + name: nexus-operator-metrics-reader +rules: + - nonResourceURLs: + - /metrics + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: nexus-operator-leader-election-rolebinding + namespace: nexus-operator-system +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: nexus-operator-leader-election-role +subjects: + - kind: ServiceAccount + name: default + namespace: nexus-operator-system +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: nexus-operator-manager-rolebinding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: nexus-operator-manager-role +subjects: + - kind: ServiceAccount + name: default + namespace: nexus-operator-system +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: nexus-operator-proxy-rolebinding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: nexus-operator-proxy-role +subjects: + - kind: ServiceAccount + name: default + namespace: nexus-operator-system +--- +apiVersion: v1 +kind: Service +metadata: + labels: + control-plane: controller-manager + name: nexus-operator-controller-manager-metrics-service + namespace: nexus-operator-system +spec: + ports: + - name: https + port: 8443 + targetPort: https + selector: + control-plane: controller-manager +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + control-plane: controller-manager + name: nexus-operator-controller-manager + namespace: nexus-operator-system +spec: + replicas: 1 + selector: + matchLabels: + control-plane: controller-manager + template: + metadata: + labels: + control-plane: controller-manager + spec: + containers: + - args: + - --secure-listen-address=0.0.0.0:8443 + - --upstream=http://127.0.0.1:8080/ + - --logtostderr=true + - --v=10 + image: gcr.io/kubebuilder/kube-rbac-proxy:v0.5.0 + name: kube-rbac-proxy + ports: + - containerPort: 8443 + name: https + - args: + - --metrics-addr=127.0.0.1:8080 + - --enable-leader-election + command: + - /manager + image: quay.io/m88i/nexus-operator:0.5.0 + name: manager + ports: + - containerPort: 9443 + name: webhook-server + protocol: TCP + resources: + requests: + cpu: 100m + memory: 20Mi + env: + - name: USE_WEBHOOKS + value: 'FALSE' + terminationGracePeriodSeconds: 10 +--- +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + labels: + control-plane: controller-manager + name: nexus-operator-controller-manager-metrics-monitor + namespace: nexus-operator-system +spec: + endpoints: + - path: /metrics + port: https + selector: + matchLabels: + control-plane: controller-manager From cb1732ad4b6728695e3788c4c9202ace60a1dc45 Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Tue, 2 Feb 2021 00:38:10 -0300 Subject: [PATCH 08/10] Fallback to in-reconcile admission if webhooks are not in use Signed-off-by: Lucas Caparelli --- CONTRIBUTING.md | 17 ++++++++++++----- config/manager/kustomization.yaml | 4 ++-- controllers/nexus_controller.go | 11 +++++++++++ main.go | 13 +++++++++---- pkg/util/env.go | 9 +++++++-- 5 files changed, 41 insertions(+), 13 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c935fbce..24ec3ba5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,11 +2,13 @@ ## Have found a bug or have a feature request? -Please, open an issue for us. That will really help us improving the Operator and it would benefit other users such as yourself. There are templates ready for bug reporting and feature request to make things easier for you. +Please, open an issue for us. That will really help us improving the Operator and it would benefit other users such as +yourself. There are templates ready for bug reporting and feature request to make things easier for you. ## Have any questions? -We're happy to answer! Either [open an issue](https://github.com/m88i/nexus-operator/issues) or send an email to our mailing list: [nexus-operator@googlegroups.com](mailto:nexus-operator@googlegroups.com). +We're happy to answer! Either [open an issue](https://github.com/m88i/nexus-operator/issues) or send an email to our +mailing list: [nexus-operator@googlegroups.com](mailto:nexus-operator@googlegroups.com). ## Are you willing to send a PR? @@ -43,9 +45,11 @@ Pushing to origin/pr-prep # (output omitted) ``` -If you don't inform remote name and branch, it will use "origin" as the remote and your current branch (the defaults, which appear between "[]"). Double check if the information is correct. +If you don't inform remote name and branch, it will use "origin" as the remote and your current branch (the defaults, +which appear between "[]"). Double check if the information is correct. -If you don't want to go over the interactive prompt every time, you can push with the defaults using the `PUSH_WITH_DEFAULTS` environment variable: +If you don't want to go over the interactive prompt every time, you can push with the defaults using the +`PUSH_WITH_DEFAULTS` environment variable: ```shell $ PUSH_WITH_DEFAULTS=TRUE make pr-prep @@ -62,4 +66,7 @@ case to `controllers/nexus_controller_test.go`. All mutating (including default values) and validating logic lives in our admission webhooks, which are not in place when the e2e suite runs. Because of that, no changes or checks are performed against Nexus CRs being reconciled. Be sure -to always write tests which use valid Nexus resources. \ No newline at end of file +to always write tests which use valid Nexus resources. + +In the future, after we migrate to operator-sdk v1.3.0 and kubebuilder plugin v3, scaffolding will create a separate +suite for testing the webhooks. \ No newline at end of file diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index ad13e96b..6a0cd6c1 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -4,5 +4,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization images: - name: controller - newName: controller - newTag: latest + newName: quay.io/m88i/nexus-operator + newTag: 0.5.0 diff --git a/controllers/nexus_controller.go b/controllers/nexus_controller.go index c4507c56..68b6c729 100644 --- a/controllers/nexus_controller.go +++ b/controllers/nexus_controller.go @@ -43,6 +43,7 @@ import ( "github.com/m88i/nexus-operator/pkg/cluster/kubernetes" "github.com/m88i/nexus-operator/pkg/cluster/openshift" "github.com/m88i/nexus-operator/pkg/framework" + "github.com/m88i/nexus-operator/pkg/util" ) const ( @@ -89,12 +90,22 @@ func (r *NexusReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return result, nil } // Error reading the object - requeue the request. + result.Requeue = true return result, err } // In case of any errors from here, we should update the Nexus CR and its status defer r.updateNexus(nexus, &err) + // if we're not using webhooks let's validate during reconcile as a fallback strategy + if !util.ShouldUseWebhooks() { + nexus.Default() + // validation is the same for create and update, any would be ok + if err := nexus.ValidateCreate(); err != nil { + return result, err + } + } + // Initialize the resource managers err = r.Supervisor.InitManagers(nexus) if err != nil { diff --git a/main.go b/main.go index 59f7eedc..dbfcf79f 100644 --- a/main.go +++ b/main.go @@ -35,6 +35,7 @@ import ( "github.com/m88i/nexus-operator/controllers" "github.com/m88i/nexus-operator/controllers/nexus/resource" "github.com/m88i/nexus-operator/pkg/cluster/discovery" + "github.com/m88i/nexus-operator/pkg/util" // +kubebuilder:scaffold:imports ) @@ -97,9 +98,13 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "Nexus") os.Exit(1) } - if err = (&appsm88iiov1alpha1.Nexus{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "Nexus") - os.Exit(1) + + if util.ShouldUseWebhooks() { + setupLog.Info("Setting up admission webhooks") + if err = (&appsm88iiov1alpha1.Nexus{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "Nexus") + os.Exit(1) + } } // +kubebuilder:scaffold:builder @@ -119,7 +124,7 @@ func getWatchNamespace() string { ns, found := os.LookupEnv(watchNamespaceEnvVar) if !found { - setupLog.Info("unable to get WatchNamespace, "+ + setupLog.Info("Unable to get WatchNamespace, "+ "the manager will watch and manage resources in all namespaces", "Env Var lookup", watchNamespaceEnvVar) return "" diff --git a/pkg/util/env.go b/pkg/util/env.go index af717f39..0deef3bb 100644 --- a/pkg/util/env.go +++ b/pkg/util/env.go @@ -19,9 +19,14 @@ import ( "strconv" ) +// ShouldUseWebhooks returns true if "USE_WEBHOOKS" value evaluates to true or if it isn't set +func ShouldUseWebhooks() bool { + return GetBoolOSEnv("USE_WEBHOOKS", "true") +} + // GetBoolOSEnv gets a env variable as a boolean -func GetBoolOSEnv(key string) bool { - val := GetOSEnv(key, "false") +func GetBoolOSEnv(key, fallback string) bool { + val := GetOSEnv(key, fallback) ret, err := strconv.ParseBool(val) if err != nil { return false From 428388b690afa872fb3af75303eefae64393e1db Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Wed, 3 Feb 2021 21:06:10 -0300 Subject: [PATCH 09/10] Make webhookless installer generator work on CI Signed-off-by: Lucas Caparelli --- .../nexus-operator-integration-checks.yaml | 22 ++++++++++++++ Makefile | 7 ++--- hack/generate-webhookless-installer.sh | 30 +++++++++++++++++++ webhookless-nexus-operator.yaml | 21 +++++++------ 4 files changed, 64 insertions(+), 16 deletions(-) create mode 100755 hack/generate-webhookless-installer.sh diff --git a/.github/workflows/nexus-operator-integration-checks.yaml b/.github/workflows/nexus-operator-integration-checks.yaml index 69eda8b8..33e6eae1 100644 --- a/.github/workflows/nexus-operator-integration-checks.yaml +++ b/.github/workflows/nexus-operator-integration-checks.yaml @@ -31,6 +31,13 @@ jobs: with: go-version: ${{ env.GO_VERSION }} id: go + - name: Setup Python + uses: actions/setup-python@v1 + with: + python-version: 3.7 + - name: Install yq + run: | + pip install yq - name: Check Vet run: | make generate @@ -66,6 +73,14 @@ jobs: restore-keys: | ${{ runner.os }}-go-cache- + - name: Setup Python + uses: actions/setup-python@v1 + with: + python-version: 3.7 + - name: Install yq + run: | + pip install yq + - name: Cache Operator SDK uses: actions/cache@v2 with: @@ -131,6 +146,13 @@ jobs: key: ${{ runner.os }}-go-${{ env.GO_VERSION }} restore-keys: | ${{ runner.os }}-go-${{ env.GO_VERSION }} + - name: Setup Python + uses: actions/setup-python@v1 + with: + python-version: 3.7 + - name: Install yq + run: | + pip install yq - name: Install Operator SDK run: | ./hack/ci/install-operator-sdk.sh diff --git a/Makefile b/Makefile index 0241f47c..300d8737 100644 --- a/Makefile +++ b/Makefile @@ -162,9 +162,6 @@ run_with_image=true pr-prep: CREATE_NAMESPACE=$(create_namespace) RUN_WITH_IMAGE=$(run_with_image) ./hack/pr-prep.sh +# Generate the installer without webhook configs, secrets and what not generate-webhookless-installer: - # first, let's filter out all manifests we don't care about - # then delete the volumes which would contain the certs - # then finally insert the env var which disables webhooks - # TODO : find a way to make this more readable - kustomize build config/default/ | yq -Y 'select(.kind != "ValidatingWebhookConfiguration" and .kind != "Issuer" and .kind != "Certificate" and .kind != "MutatingWebhookConfiguration" and .metadata.name != "nexus-operator-webhook-service")' | yq -Y 'del(.. | .volumes?, .volumeMounts?)' | yq -Y 'if .kind=="Deployment" then .spec.template.spec.containers[1].env[0]={"name":"USE_WEBHOOKS", "value":"FALSE"} else . end' > webhookless-nexus-operator.yaml + ./hack/generate-webhookless-installer.sh diff --git a/hack/generate-webhookless-installer.sh b/hack/generate-webhookless-installer.sh new file mode 100755 index 00000000..77b2e2e6 --- /dev/null +++ b/hack/generate-webhookless-installer.sh @@ -0,0 +1,30 @@ +#!/bin/bash +# Copyright 2020 Nexus Operator and/or its 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. + +command -v yq > /dev/null || ( echo "Please install yq before proceeding (https://pypi.org/project/yq/)" && exit 1 ) +command -v kustomize > /dev/null || go build -o "${GOPATH}"/bin/kustomize sigs.k8s.io/kustomize/kustomize/v3@v3.5.4 || exit 1 + +# first, let's filter out all manifests for kinds we don't care about +# 'select(.kind != "ValidatingWebhookConfiguration" and .kind != "Issuer" and .kind != "Certificate" and .kind != "MutatingWebhookConfiguration" and .metadata.name != "nexus-operator-webhook-service")' + +# then delete the volumes which would contain the certs +# 'del(.. | .volumes?, .volumeMounts?)' + +# then finally insert the env var which disables webhooks +# 'if .kind=="Deployment" then .spec.template.spec.containers[1].env[0]={"name":"USE_WEBHOOKS", "value":"FALSE"} else . end' + +kustomize build config/default/ | yq -Y 'select(.kind != "ValidatingWebhookConfiguration" and .kind != "Issuer" and .kind != "Certificate" and .kind != "MutatingWebhookConfiguration" and .metadata.name != "nexus-operator-webhook-service")' \ + | yq -Y 'del(.. | .volumes?, .volumeMounts?)' \ + | yq -Y 'if .kind=="Deployment" then .spec.template.spec.containers[1].env[0]={"name":"USE_WEBHOOKS", "value":"FALSE"} else . end' > webhookless-nexus-operator.yaml diff --git a/webhookless-nexus-operator.yaml b/webhookless-nexus-operator.yaml index 4d0506c2..e78a6bab 100644 --- a/webhookless-nexus-operator.yaml +++ b/webhookless-nexus-operator.yaml @@ -11,7 +11,6 @@ metadata: annotations: cert-manager.io/inject-ca-from: nexus-operator-system/nexus-operator-serving-cert controller-gen.kubebuilder.io/version: v0.3.0 - creationTimestamp: null name: nexus.apps.m88i.io spec: additionalPrinterColumns: @@ -694,16 +693,6 @@ spec: control-plane: controller-manager spec: containers: - - args: - - --secure-listen-address=0.0.0.0:8443 - - --upstream=http://127.0.0.1:8080/ - - --logtostderr=true - - --v=10 - image: gcr.io/kubebuilder/kube-rbac-proxy:v0.5.0 - name: kube-rbac-proxy - ports: - - containerPort: 8443 - name: https - args: - --metrics-addr=127.0.0.1:8080 - --enable-leader-election @@ -719,6 +708,16 @@ spec: requests: cpu: 100m memory: 20Mi + - args: + - --secure-listen-address=0.0.0.0:8443 + - --upstream=http://127.0.0.1:8080/ + - --logtostderr=true + - --v=10 + image: gcr.io/kubebuilder/kube-rbac-proxy:v0.5.0 + name: kube-rbac-proxy + ports: + - containerPort: 8443 + name: https env: - name: USE_WEBHOOKS value: 'FALSE' From acd762c8c93867c6309dc906ccc92d91918b317b Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Wed, 3 Feb 2021 22:11:48 -0300 Subject: [PATCH 10/10] Add and document dependency on cert-manager Signed-off-by: Lucas Caparelli --- README.md | 45 +++++++++++++++++++++++++++---- RELEASE_NOTES.md | 7 +++-- bundle/metadata/dependencies.yaml | 20 ++++++++++++++ webhookless-nexus-operator.yaml | 21 ++++++++------- 4 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 bundle/metadata/dependencies.yaml diff --git a/README.md b/README.md index a14f7aa5..6a0ae02f 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ Table of Contents * [Table of Contents](#table-of-contents) * [Nexus Operator](#nexus-operator) * [Pre Requisites](#pre-requisites) + * [Kubernetes API dependencies](#kubernetes-api-dependencies) * [Quick Install](#quick-install) * [Openshift](#openshift) * [Clean up](#clean-up) @@ -20,8 +21,8 @@ Table of Contents * [Networking](#networking) * [Use NodePort](#use-nodeport) * [Network on OpenShift](#network-on-openshift) - * [Network on Kubernetes 1.14 ](#network-on-kubernetes-114) - * [NGINX Ingress troubleshooting](#nginx-ingress-troubleshooting) + * [Network on Kubernetes 1.14+](#network-on-kubernetes-114) + * [NGINX Ingress troubleshooting](#nginx-ingress-troubleshooting) * [TLS/SSL](#tlsssl) * [Persistence](#persistence) * [Minikube](#minikube) @@ -41,9 +42,14 @@ Table of Contents A Nexus OSS Kubernetes Operator based on the [Operator SDK](https://github.com/operator-framework/operator-sdk). -You can find us at [OperatorHub](https://operatorhub.io/operator/nexus-operator-m88i) or at the ["Operators" tab in your OpenShift 4.x web console](https://docs.openshift.com/container-platform/4.4/operators/olm-adding-operators-to-cluster.html), just search for "Nexus". If you don't have access to [OLM](https://github.com/operator-framework/operator-lifecycle-manager), try installing it manually [following our quick installation guide](#quick-install). +You can find us at [OperatorHub](https://operatorhub.io/operator/nexus-operator-m88i) or at +the ["Operators" tab in your OpenShift 4.x web console](https://docs.openshift.com/container-platform/4.4/operators/olm-adding-operators-to-cluster.html) +, just search for "Nexus". If you don't have access +to [OLM](https://github.com/operator-framework/operator-lifecycle-manager), try installing it +manually [following our quick installation guide](#quick-install). -If you have any questions please either [open an issue](https://github.com/m88i/nexus-operator/issues) or send an email to the mailing list: [nexus-operator@googlegroups.com](mailto:nexus-operator@googlegroups.com). +If you have any questions please either [open an issue](https://github.com/m88i/nexus-operator/issues) or send an email +to the mailing list: [nexus-operator@googlegroups.com](mailto:nexus-operator@googlegroups.com). ## Pre Requisites @@ -51,9 +57,38 @@ If you have any questions please either [open an issue](https://github.com/m88i/ - Kubernetes or OpenShift cluster available (minishift, minikube or crc also supported) - Cluster admin credentials to install the Operator +### Kubernetes API dependencies + +Starting on v0.6.0, the Operator relies on [cert-manager](https://cert-manager.io/) for generating and injecting the TLS +certificate for +the [admission webhooks](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#admission-webhooks) +we use. When installing the Operator via OLM (e.g., following instruction +on [Operator Hub](https://operatorhub.io/operator/nexus-operator-m88i)), cert-manager will be automatically installed in +your cluster. When installing using other methods, it's necessary +to [manually install it](https://cert-manager.io/docs/installation/). + +However, if you simply want to try out the operator without webhooks, you may install it with: + +```bash +VERSION= + +kubectl apply -f https://github.com/m88i/nexus-operator/releases/download/${VERSION}/webhookless-nexus-operator.yaml +``` + +We strongly advise using webhooks as they significantly improve user experience by reporting validation errors +instantly (which you would need to look for in the logs otherwise). They also persist any changes made to the Nexus +resource (such as default values), which would otherwise only happen during runtime (opaque to users). + +Additionally, there is also a weak dependency on +the [Prometheus Operator's](https://github.com/prometheus-operator/prometheus-operator) `ServiceMonitor` CRD. The +Operator will run fine without it, but if this API is available it will be used to improve monitoring. For more +information check their [CRDs doc](https://github.com/prometheus-operator/prometheus-operator#customresourcedefinitions) +and [quickstart guide](https://github.com/prometheus-operator/prometheus-operator#quickstart). + ## Quick Install -The installation procedure will create a Namespace named `nexus-operator-system` and will install every resources needed for the operator to run: +The installation procedure will create a Namespace named `nexus-operator-system` and will install every resources needed +for the operator to run: ```bash # requires python and kubectl diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a8308b08..f57c88de 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,8 +1,7 @@ -## Version 0.5.0 +## Version 0.6.0 ### Enhancements -- #198 - Upgrade to Go v1.15.6 -- #199 - Upgrade to operator-sdk v1.2.0 + +- # 174 - Use Admission Webhooks to set default values and validate to Nexus CR ### Bug Fixes -- #191 - Pod fails to start after modifying the Nexus resource diff --git a/bundle/metadata/dependencies.yaml b/bundle/metadata/dependencies.yaml new file mode 100644 index 00000000..c88cb811 --- /dev/null +++ b/bundle/metadata/dependencies.yaml @@ -0,0 +1,20 @@ +# https://olm.operatorframework.io/docs/concepts/olm-architecture/dependency-resolution/ +dependencies: + # ideally, this would be an optional/weak dep, the operator still works fine without it + # doesn't look like it will be implemented any time soon though https://github.com/operator-framework/operator-lifecycle-manager/issues/819 + # if this situation changes, just uncomment the following lines + # - type: olm.gvk + # value: + # group: monitoring.coreos.com + # kind: ServiceMonitor + # version: v1 + - type: olm.gvk + value: + group: cert-manager.io + kind: Certificate + version: v1alpha2 + - type: olm.gvk + value: + group: cert-manager.io + kind: Issuer + version: v1alpha2 diff --git a/webhookless-nexus-operator.yaml b/webhookless-nexus-operator.yaml index e78a6bab..4d0506c2 100644 --- a/webhookless-nexus-operator.yaml +++ b/webhookless-nexus-operator.yaml @@ -11,6 +11,7 @@ metadata: annotations: cert-manager.io/inject-ca-from: nexus-operator-system/nexus-operator-serving-cert controller-gen.kubebuilder.io/version: v0.3.0 + creationTimestamp: null name: nexus.apps.m88i.io spec: additionalPrinterColumns: @@ -693,6 +694,16 @@ spec: control-plane: controller-manager spec: containers: + - args: + - --secure-listen-address=0.0.0.0:8443 + - --upstream=http://127.0.0.1:8080/ + - --logtostderr=true + - --v=10 + image: gcr.io/kubebuilder/kube-rbac-proxy:v0.5.0 + name: kube-rbac-proxy + ports: + - containerPort: 8443 + name: https - args: - --metrics-addr=127.0.0.1:8080 - --enable-leader-election @@ -708,16 +719,6 @@ spec: requests: cpu: 100m memory: 20Mi - - args: - - --secure-listen-address=0.0.0.0:8443 - - --upstream=http://127.0.0.1:8080/ - - --logtostderr=true - - --v=10 - image: gcr.io/kubebuilder/kube-rbac-proxy:v0.5.0 - name: kube-rbac-proxy - ports: - - containerPort: 8443 - name: https env: - name: USE_WEBHOOKS value: 'FALSE'