diff --git a/go.mod b/go.mod index 4b7b57e..e6d0dc1 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,9 @@ require ( github.com/onsi/gomega v1.33.1 github.com/samber/lo v1.47.0 github.com/sirupsen/logrus v1.9.3 + github.com/stretchr/testify v1.9.0 + istio.io/api v1.23.2 + istio.io/client-go v1.23.2 k8s.io/api v0.31.0 k8s.io/apimachinery v0.31.0 k8s.io/client-go v0.31.0 @@ -55,6 +58,7 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.19.1 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.55.0 // indirect @@ -90,8 +94,6 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - istio.io/api v1.23.2 // indirect - istio.io/client-go v1.23.2 // indirect k8s.io/apiextensions-apiserver v0.31.0 // indirect k8s.io/apiserver v0.31.0 // indirect k8s.io/component-base v0.31.0 // indirect diff --git a/go.sum b/go.sum index 3602659..5c123f5 100644 --- a/go.sum +++ b/go.sum @@ -22,8 +22,8 @@ github.com/dominikbraun/graph v0.23.0 h1:TdZB4pPqCLFxYhdyMFb1TBdFxp8XLcJfTTBQucV github.com/dominikbraun/graph v0.23.0/go.mod h1:yOjYyogZLY1LSG9E33JWZJiq5k83Qy2C6POAuiViluc= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g= github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= -github.com/evanphx/json-patch v0.5.2 h1:xVCHIVMUu1wtM/VkR9jVZ45N3FhZfYMMYGorLCR8P3k= -github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ= +github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U= +github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg= github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= diff --git a/kardinal/reconciler/reconciler.go b/kardinal/reconciler/reconciler.go index 4e2a6e8..8994824 100644 --- a/kardinal/reconciler/reconciler.go +++ b/kardinal/reconciler/reconciler.go @@ -10,20 +10,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -const ( - // Thi is a common label used in several applications and recommended by Kubernetes: https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/ - appNameKubernetesLabelKey = "app.kubernetes.io/name" - appLabelKey = "app" - versionLabelKey = "version" - defaultVersionLabelValue = "baseline" -) - -type labeledResources interface { - GetLabels() map[string]string - SetLabels(labels map[string]string) - GetName() string -} - func Reconcile(ctx context.Context, cl client.Client) error { logrus.Info("Reconciling") @@ -34,8 +20,8 @@ func Reconcile(ctx context.Context, cl client.Client) error { return stacktrace.Propagate(err, "An error occurred retrieving the list of resources") } - if err := reconcileIstioLabelsForBaselineServicesAndDeployments(ctx, cl, clusterResources); err != nil { - return stacktrace.Propagate(err, "An error occurred reconciling the Istio labels") + if err := resources.InjectIstioLabelsInServicesAndDeployments(ctx, cl, clusterResources); err != nil { + return stacktrace.Propagate(err, "An error occurred injecting the Istio labels in the services and deployments") } // Generate base cluster topology @@ -113,56 +99,3 @@ func Reconcile(ctx context.Context, cl client.Client) error { return nil } - -// OPERATOR-TODO make sure to execute this again once we connect the operator to listen to k8s Deployments and Services events -// OPERATOR-TODO there is another approach we could take, if it doesn't works for all use cases, which is to use MutatingAdmissionWebHooks -// related info for this here: https://book.kubebuilder.io/cronjob-tutorial/webhook-implementation and particularly this https://book.kubebuilder.io/reference/webhook-for-core-types -// for creating and webhook for these core types. -func reconcileIstioLabelsForBaselineServicesAndDeployments(ctx context.Context, cl client.Client, clusterResources *resources.Resources) error { - for _, namespace := range clusterResources.Namespaces { - for _, service := range namespace.Services { - ensureIstioLabelsForResource(service) - if err := cl.Update(ctx, service); err != nil { - return stacktrace.Propagate(err, "An error occurred adding Istio labels to service '%s'", service.GetName()) - } - } - - for _, deployment := range namespace.Deployments { - ensureIstioLabelsForResource(deployment) - if err := cl.Update(ctx, deployment); err != nil { - return stacktrace.Propagate(err, "An error occurred adding Istio labels to deployment '%s'", deployment.GetName()) - } - } - } - return nil -} - -func ensureIstioLabelsForResource(resource labeledResources) error { - - labels := resource.GetLabels() - if labels == nil { - labels = map[string]string{} - } - - // The 'app' label - _, ok := labels[appLabelKey] - if !ok { - appNameKubernetesLabelValue, ok := labels[appNameKubernetesLabelKey] - if ok { - labels[appLabelKey] = appNameKubernetesLabelValue - } else { - labels[appLabelKey] = resource.GetName() - } - } - - // The 'version' label - // OPERATOR-TODO how are we going to handle when a non-managed resource already has the "version" label and - // this value is different from the value needed for managing the baseline traffic - _, ok = labels[versionLabelKey] - if !ok { - labels[versionLabelKey] = defaultVersionLabelValue - } - resource.SetLabels(labels) - - return nil -} diff --git a/kardinal/resources/resources.go b/kardinal/resources/resources.go index 30fc647..611f762 100644 --- a/kardinal/resources/resources.go +++ b/kardinal/resources/resources.go @@ -18,11 +18,23 @@ import ( ) const ( - BaselineNamespace = "baseline" - trueStr = "true" - kardinalManagedLabelKey = "kardinal.dev/managed" + BaselineNamespace = "baseline" + defaultVersionLabelValue = "baseline" + trueStr = "true" + kardinalManagedLabelKey = "kardinal.dev/managed" + + // Thi is a common label used in several applications and recommended by Kubernetes: https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/ + appNameKubernetesLabelKey = "app.kubernetes.io/name" + appLabelKey = "app" + versionLabelKey = "version" ) +type labeledResources interface { + GetLabels() map[string]string + SetLabels(labels map[string]string) + GetName() string +} + type Resources struct { Namespaces []*Namespace } @@ -370,3 +382,67 @@ func ApplyIngressResources(ctx context.Context, clusterResources *Resources, clu return nil } + +// OPERATOR-TODO make sure to execute this again once we connect the operator to listen to k8s Deployments and Services events +// OPERATOR-TODO there is another approach we could take, if it doesn't works for all use cases, which is to use MutatingAdmissionWebHooks +// related info for this here: https://book.kubebuilder.io/cronjob-tutorial/webhook-implementation and particularly this https://book.kubebuilder.io/reference/webhook-for-core-types +// for creating and webhook for these core types. +func InjectIstioLabelsInServicesAndDeployments(ctx context.Context, cl client.Client, clusterResources *Resources) error { + for _, namespace := range clusterResources.Namespaces { + for _, service := range namespace.Services { + shouldUpdateLabels := ensureIstioLabelsForResource(service) + if shouldUpdateLabels { + if err := cl.Update(ctx, service); err != nil { + return stacktrace.Propagate(err, "An error occurred adding Istio labels to service '%s'", service.GetName()) + } + } + + } + + for _, deployment := range namespace.Deployments { + shouldUpdateLabels := ensureIstioLabelsForResource(deployment) + if shouldUpdateLabels { + if err := cl.Update(ctx, deployment); err != nil { + return stacktrace.Propagate(err, "An error occurred adding Istio labels to deployment '%s'", deployment.GetName()) + } + } + } + } + return nil +} + +func ensureIstioLabelsForResource(resource labeledResources) bool { + + var areLabelsUpdated bool + + labels := resource.GetLabels() + if labels == nil { + labels = map[string]string{} + } + + // The 'app' label + _, ok := labels[appLabelKey] + if !ok { + areLabelsUpdated = true + appNameKubernetesLabelValue, ok := labels[appNameKubernetesLabelKey] + if ok { + labels[appLabelKey] = appNameKubernetesLabelValue + } else { + labels[appLabelKey] = resource.GetName() + } + } + + // The 'version' label + // OPERATOR-TODO how are we going to handle when a non-managed resource already has the "version" label and + // this value is different from the value needed for managing the baseline traffic + _, ok = labels[versionLabelKey] + if !ok { + areLabelsUpdated = true + labels[versionLabelKey] = defaultVersionLabelValue + } + if areLabelsUpdated { + resource.SetLabels(labels) + } + + return areLabelsUpdated +} diff --git a/kardinal/resources/resources_test.go b/kardinal/resources/resources_test.go new file mode 100644 index 0000000..08d0973 --- /dev/null +++ b/kardinal/resources/resources_test.go @@ -0,0 +1,81 @@ +package resources + +import ( + "github.com/stretchr/testify/require" + "testing" +) + +type labeledResourcesForTest struct { + labels map[string]string + name string +} + +func newLabeledResourcesForTest(labels map[string]string, name string) *labeledResourcesForTest { + return &labeledResourcesForTest{labels: labels, name: name} +} + +func (l *labeledResourcesForTest) GetLabels() map[string]string { + return l.labels +} + +func (l *labeledResourcesForTest) SetLabels(labels map[string]string) { + l.labels = labels +} + +func (l *labeledResourcesForTest) GetName() string { + return l.name +} + +func TestIstioLabelsEnsurer(t *testing.T) { + + withoutAnyIstioLabel := map[string]string{} + labeledResourceWithoutAnyIstioLabel := newLabeledResourcesForTest(withoutAnyIstioLabel, "my-service") + expectedLabels := map[string]string{ + "app": "my-service", + "version": "baseline", + } + + expectedBool := ensureIstioLabelsForResource(labeledResourceWithoutAnyIstioLabel) + require.True(t, expectedBool) + require.Equal(t, expectedLabels, labeledResourceWithoutAnyIstioLabel.GetLabels()) + + withK8sAppLabel := map[string]string{ + "app.kubernetes.io/name": "my-service-using-k8s-app", + } + labeledResourceWithK8sAppLabel := newLabeledResourcesForTest(withK8sAppLabel, "my-service") + expectedLabels = map[string]string{ + "app.kubernetes.io/name": "my-service-using-k8s-app", + "app": "my-service-using-k8s-app", + "version": "baseline", + } + expectedBool = ensureIstioLabelsForResource(labeledResourceWithK8sAppLabel) + require.True(t, expectedBool) + require.Equal(t, expectedLabels, labeledResourceWithK8sAppLabel.GetLabels()) + + withVersionLabel := map[string]string{ + "version": "v0.2.1", + } + labeledResourceWithVersionLabel := newLabeledResourcesForTest(withVersionLabel, "my-versioned-service") + expectedLabels = map[string]string{ + "app": "my-versioned-service", + "version": "v0.2.1", + } + + expectedBool = ensureIstioLabelsForResource(labeledResourceWithVersionLabel) + require.True(t, expectedBool) + require.Equal(t, expectedLabels, labeledResourceWithVersionLabel.GetLabels()) + + withVersionAndAppLabel := map[string]string{ + "app": "my-app", + "version": "v0.2.1", + } + labeledResourceWithVersionAndAppLabel := newLabeledResourcesForTest(withVersionAndAppLabel, "my-app-service") + expectedLabels = map[string]string{ + "app": "my-app", + "version": "v0.2.1", + } + + expectedBool = ensureIstioLabelsForResource(labeledResourceWithVersionAndAppLabel) + require.False(t, expectedBool) + require.Equal(t, expectedLabels, labeledResourceWithVersionAndAppLabel.GetLabels()) +}