Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configure SKR event listener path more explicitly #1940

Open
5 tasks
c-pius opened this issue Oct 8, 2024 · 0 comments
Open
5 tasks

Configure SKR event listener path more explicitly #1940

c-pius opened this issue Oct 8, 2024 · 0 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@c-pius
Copy link
Contributor

c-pius commented Oct 8, 2024

Description

With #1822, we changed the label the runtime-watcher is watching for from lifecycle-manger to kyma. There was some confusion about whether this also changes the endpoint the change events are sent to / received on at KCP side. As of now, the state is as follows:

Given, the following WatcherCR:

apiVersion: operator.kyma-project.io/v1beta2
kind: Watcher
metadata:
  name: watcher
  labels:
    "operator.kyma-project.io/managed-by": "lifecycle-manager"
spec:
  labelsToWatch:
    "operator.kyma-project.io/watched-by": "kyma"

The labelsToWatch entry "operator.kyma-project.io/watched-by": "kyma" defines what labels must be present on an SKR resource for being watched. As of now, this is set to kyma which means that we are watching resources having the label "operator.kyma-project.io/watched-by": "kyma". The ValidatingWebhookConfiguration on the SKR will be created with .webhooks[].objectSelector.matchLabels."operator.kyma-project.io/watched-by": kyma.

The .metadata.labels entry "operator.kyma-project.io/managed-by": "lifecycle-manager" defines where those watch events are routed to. As of now, this is set to lifecycle-manager which means that we are routing events to /v1/lifecycle-manager/events. The ValidatingWebhookConfiguration on the SKR will be created with .webhooks[].admissionReviewVersions[].clientConfig.service.path: /validate/lifecycle-manager (see svcPath var in pkg/watcher/skr_webhook_resources.go). Consequently, requests are sent to runtime-watcher (handler) with path /validate/lifecycle-manager. The handler parses the lifecycle-manager part (see moduleName var) and uses it to construct the path to send the requests to, i.e., /v1/lifecycle-manager/event.

At KCP side, KLM sets the VirtualService .spec.http.match.uri.prefix according to the WatcherCRs .metadata.labels entry "operator.kyma-project.io/managed-by": "lifecycle-manager" (see internal/istio/httproute.go). Additionally, the KLM listener, must be configured to use the same lifecycle-manager value. This is currently done in main.go by:

runnableListener := watcherevent.NewSKREventListener(
		settings.ListenerAddr,
		shared.OperatorName, // <-- constant: lifecycle-manager
		verifyFunc,
	)

So the Watchers .metadata.labels entry "operator.kyma-project.io/managed-by": "lifecycle-manager" and this code config must match.


From technical perspective, the label to watch and the path those watch requests are sent to are decoupled already. We should however make this configuration more explicit. Using the watched-by label still appears reasonable. Using the WatcherCR managed-by label to define the listener address that the events are sent to appears suboptimal. We should think about how we can make this mapping configuration more explicit. Ideally, we also have a solution with a single source of truth (right now we need to configure it at two locations: label and watcherevent.NewSKREventListener param).

UPDATE: after thinking, it may be reasonable or even required to have sperate configs for the endpoint. The WatcherCR defines for SKR where to send the events to. The rest is for the actual listener setup in KCP, which may also be completely outside of the KLM realm.

Reasons

  • reduce chances of another misconfiguration in the future

Acceptance Criteria

  • proposed and impelmented a config solution that make this mapping from watched-by label to listener endpoint more explicit
    • solution ideally has one single source of truth
  • solution supports the use case of module team defining their own watched-by label value and routing it to a module-team defined endpoint
    • TBC: must the endpoint be within KCP?
  • document the configuration and how to use it properly
  • side quest: consider to make runtime-watcher log the path the event is sent to

Feature Testing

No response

Testing approach

No response

Attachments

No response

@c-pius c-pius added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

1 participant