Multi cluster support for Ingress/httproute discovery

[gberche-orange]

Thanks for this great work and welcoming new ideas in #25 (comment) !

In our project, we need to include ingresses from multiple k8s clusters into our homer dashboard, not only the cluster where the homer-operator is deployed. This enables using the homer features (such as search) on discovered ingresses/routes from multiple clusters. The alternative of deploying homer-operator is each k8s cluster would not enable such global search for all ingresses/routes.

The suggestion would be that homer operator accepts a list of clusters to perform the discovery on. This could be a an array of secret references, pointing to secret containing kubeconfig.

See https://fluxcd.io/flux/components/kustomize/kustomizations/#kubeconfig-remote-clusters for inspiration on how fluxcd kustomize controller supports specifying a remote cluster kubeconfig

.spec.kubeConfig.secretRef: Secret-based authentication using a static kubeconfig stored in a Kubernetes Secret in the same namespace as the Kustomization.

---
apiVersion: v1
kind: Secret
metadata:
  name: prod-kubeconfig
type: Opaque
stringData:
  value.yaml: |
    apiVersion: v1
    kind: Config
    # ...omitted for brevity    

Note: The KubeConfig should be self-contained and not rely on binaries, environment, or credential files from the kustomize-controller Pod.

Currently, the service discovery is performed by a single service account projected token interacting with the current cluster

homer-operator/charts/homer-operator/values.yaml

Lines 51 to 55 in f61a096

	serviceAccount:
	create: true
	automount: true
	annotations: {}
	name: ""

homer-operator/charts/homer-operator/templates/serviceaccount.yaml

Lines 1 to 15 in f61a096

	{{- if .Values.serviceAccount.create -}}
	---
	apiVersion: v1
	kind: ServiceAccount
	metadata:
	name: {{ include "homer-operator.serviceAccountName" . }}
	namespace: {{ include "homer-operator.namespace" . }}
	labels:
	{{- include "homer-operator.labels" . | nindent 4 }}
	{{- with .Values.serviceAccount.annotations }}
	annotations:
	{{- toYaml . | nindent 4 }}
	{{- end }}
	automountServiceAccountToken: {{ .Values.serviceAccount.automount }}
	{{- end }}

which injects the service account using service account token projection https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#launch-a-pod-using-service-account-token-projection

homer-operator/charts/homer-operator/templates/deployment.yaml

Line 40 in f61a096

serviceAccountName: {{ include "homer-operator.serviceAccountName" . }}

The new feature would require to create for each remote k8s clusters, new k8s clients and ingress/httproute controllers for the service discovery, whereas current code creates a single client and ingress/httproute controllers

homer-operator/cmd/main.go

Lines 116 to 134 in f61a096

	ingressController := &controller.GenericResourceReconciler{
	Client: mgr.GetClient(),
	Scheme: mgr.GetScheme(),
	}
	if err = ingressController.SetupIngressController(mgr); err != nil {
	setupLog.Error(err, "unable to create controller", "controller", "Ingress")
	os.Exit(1)
	}
	if enableGatewayAPI {
	httpRouteController := &controller.GenericResourceReconciler{
	Client: mgr.GetClient(),
	Scheme: mgr.GetScheme(),
	}
	if err = httpRouteController.SetupHTTPRouteController(mgr); err != nil {
	setupLog.Error(err, "unable to create controller", "controller", "HTTPRoute")
	os.Exit(1)
	}
	}

[rajsinghtech]

Done, Try it out!

[gberche-orange]

Awesome, thanks a lot @rajsinghtech ! We'll be testing and provide feedback.

In the meantime, some feedback quickly reviewing 1af8945

I wonder whether the enableDependencyAnalysis is a feature already implemented or rather a place holder for future support ?

homer-operator/config/samples/homer_v1alpha1_dashboard_multicluster.yaml

Line 191 in 1af8945

enableDependencyAnalysis: true

I could not yet find actual usage of the configuration flag in the source code https://github.com/search?q=repo%3Arajsinghtech%2Fhomer-operator%20enableDependencyAnalysis&type=code nor in upstream homer codebase https://github.com/search?q=repo%3Abastienwirtz%2Fhomer+DependencyAnalysis&type=code

Same question for

homer-operator/config/samples/homer_v1alpha1_dashboard_multicluster.yaml

Lines 192 to 193 in 1af8945

	enableMetricsAggregation: true
	enableLayoutOptimization: true

Thanks again !

[gberche-orange]

Thanks for the fix related to #29 (comment) in aa6a525

I'd suggest reopening this issue to keep related feedback in this original issue.

When testing the multi cluster feature with @ogrand, we observe that the multi cluster discovered ingresses are missing discovered annotations in the homer configmap, we're using ingress annotations such as

homer-operator/README.md

Lines 253 to 263 in f8f87e4

	apiVersion: networking.k8s.io/v1
	kind: Ingress
	metadata:
	name: my-app-ingress
	annotations:
	# Item configuration
	item.homer.rajsingh.info/name: "My Application"
	item.homer.rajsingh.info/subtitle: "Production instance"
	item.homer.rajsingh.info/logo: "https://example.com/logo.png"
	item.homer.rajsingh.info/tag: "production"
	item.homer.rajsingh.info/keywords: "app, api, service"

I notice that the following E2E tests that cover discovered ingresses annotations for a single cluster at the following code fragments, don't yet include coverage for multi clusters.

homer-operator/test/e2e/e2e_test.go

Lines 363 to 371 in f8f87e4

	ingress := &networkingv1.Ingress{
	ObjectMeta: metav1.ObjectMeta{
	Name: "e2e-test-ingress",
	Namespace: testNs,
	Annotations: map[string]string{
	"environment": "e2e-test",
	"item.homer.rajsingh.info/name": "E2E Test App",
	"item.homer.rajsingh.info/subtitle": "End-to-End Test Application",
	"service.homer.rajsingh.info/name": "E2E Services",

homer-operator/test/e2e/e2e_test.go

Lines 429 to 432 in f8f87e4

	Expect(configYaml).To(ContainSubstring("e2e.test.local"))
	Expect(configYaml).To(ContainSubstring("E2E Test App"))
	Expect(configYaml).To(ContainSubstring("End-to-End Test Application"))
	Expect(configYaml).To(ContainSubstring("E2E Services"))

I wonder whether E2E tests could leverage the declarative chainsaw test framework available at https://kyverno.github.io/chainsaw/latest/ which show examples of multi cluster tests using kind at https://kyverno.github.io/chainsaw/latest/examples/multi-cluster/ and https://github.com/kyverno/chainsaw/blob/main/testdata/e2e/examples/dynamic-clusters/chainsaw-test.yaml triggered from gh workflows at https://github.com/kyverno/chainsaw/blob/74e7dc3296d31bfd29cb40066962d26df9b78ae1/.github/workflows/tests.yaml#L66-L83
The chainsaw assertions could check the expected homer configmap content such as in https://github.com/kyverno/chainsaw/blob/main/testdata/e2e/examples/basic/configmap-assert.yaml

I however realize that this may be too heavy work for this feature.

Thanks again for your work implementing our suggested ideas, much appreciated.

[rajsinghtech]

@gberche-orange how are things working for you now?

[gberche-orange]

sorry for late response, we're suffering from infrastructure issues which have slowed down our tests and our feedback. We hope to be able to feedback on the fix related to missing annotations in multi-cluster discovered ingresses during the week. Thanks for your patience.

[ogrand]

Hello Raj Singh, latest tests shows that annotations set on ingresses for remote clusters are not applied in the dashboard.

Only annotations on main cluster (which runs homer operator) are used and applied.

[rajsinghtech]

@ogrand @gberche-orange 0.0.13 is out try now!

[gberche-orange]

thanks @rajsinghtech ! @ogrand is out of office for a week, he'll look at 0.0.18 when he gets back.

Blackbox E2E tests such as chainsaw-based tests mentionned in #29 (comment) would help ensure that the feature is actually working as expected and won't regress in a future refactoring. Is it something you could consider ?

In my understanding, this the approach most operators take when there feature set becomes rich and valuable enough, see https://github.com/search?q=kyverno%2Faction-install-chainsaw++path%3A.github%2Fworkflows%2F*.yaml&type=code&ref=advsearch for examples of such operators adopting the chainsaw