fluxcd / notification-controller

The GitOps Toolkit event forwarder and notification dispatcher
https://fluxcd.io
Apache License 2.0
151 stars 133 forks source link

Alerts/Github - is it possible to separate add additional metadata? #589

Open audunsolemdal opened 1 year ago

audunsolemdal commented 1 year ago

So my goal is to run a github action step to check if fluxcd reconciliation for a kustomization towards a specificcluster was successful. There are three different kustomizations in three different clusters which may post reconcilation successful status to my commit on github. The kustomizations have the same exact name It does not seem like modifying the eventMetadata or summary fields in the Alert resource adds any extra metadata to the github status.

I guess a workaround is to name the kustomizations tstsp1-github-info-${cluster} rather than just tstsp1-github-info, but I am wondering if there are other methods to inject this metadata into the Github status? The description or context fields would be sufficient to modify in my use case.

Say I have a repo with the following structure

orgX/wl-tstsp1:

fluxcd/
  dev/
    deployment.yaml
  test/
    deployment.yaml
  prod/
    deployment.yaml

Each folder is connected to the namespace called tstsp1 in three different cluster (dev/test/prod) From each of the clusters there is a alert updating the github status

---
apiVersion: notification.toolkit.fluxcd.io/v1beta2
kind: Provider
metadata:
  name: tstsp1-github
  namespace: flux-system
spec:
  type: github
  address: https://github.com/orgX/wl-tstsp1
  secretRef:
    name: gh-flux-pat
---
apiVersion: notification.toolkit.fluxcd.io/v1beta2
kind: Alert
metadata:
  name: "tstsp1-github-info"
  namespace: flux-system
spec:
  providerRef:
    name: tstsp1-github
  eventSeverity: info
  summary: "dev01"
  eventMetadata:
    cluster: dev01
  eventSources:
    - kind: Kustomization
      name: '*'
      namespace: tstsp1

I end up with a status looking somewhat like the below:

gh api /repos/orgX/wl-tstsp1/statuses/88efe1eeb3e2ad9724acc6ff7c06324f6e613a66
[
  {
    "url": "https://api.github.com/repos/orgX/wl-tstsp1/statuses/88efe1eeb3e2ad9724acc6ff7c06324f6e613a66",
    "avatar_url": "https://avatars.githubusercontent.com/u/18458716?v=4",
    "id": 24342874371,
    "node_id": "SC_kwDOHpAnes8AAAAFqvLJAw",
    "state": "success",
    "description": "reconciliation succeeded",
    "target_url": null,
    "context": "kustomization/tstsp1/025ecd77",
    "created_at": "2023-07-31T10:38:27Z",
    "updated_at": "2023-07-31T10:38:27Z",
    "creator": {
      ...
    }
  }
]   
makkes commented 1 year ago

The context field of the status is constructed from the involved object and the UID of the provider (.metadata.uid of each Provider object) to make it unique across clusters but I reckon you might want something more easily trackable to the cluster the status came from.

The commit status API accepts the parameters state, description, context and target_url. The first three are already used by Flux but maybe we can make the context configurable to include an additional field derived from Alert.spec.Summary (which ends up in the event metadata).

audunsolemdal commented 1 year ago

I'll be happy with any solution that doesn't involve renaming my kustomizations per cluster. I also tried setting

eventMetadata:
  context: ${cluster}

to no avail, my understanding is that certain fields as context cannot be overridden https://github.com/fluxcd/notification-controller/pull/529/files#diff-9c161f071659a6d134b4cfdd27ad66fc88d93b5dfb102d7bb5a768b9e9e483eaR81 as you mentioned they are used by flux already.

audunsolemdal commented 1 year ago

Looks like this PR described the same use case https://github.com/fluxcd/notification-controller/pull/340

makkes commented 1 year ago

Looks like this PR described the same use case #340

I don't think a new field is necessary here as we can rather fetch the needed information from the Alert object's .spec.summary field as suggested here.

somtochiama commented 1 year ago

maybe we can make the context configurable to include an additional

@makkes maybe we should include it in the description field instead?

makkes commented 1 year ago

maybe we can make the context configurable to include an additional

@makkes maybe we should include it in the description field instead?

Would be fine from my perspective.

gdasson commented 10 months ago

@makkes : Please assign this issue to me. I can work on it.

hirenko-v commented 5 months ago

is there any workaround to achieve this?

hirenko-v commented 5 months ago

I see that ut uses uid of the provider in commit status like this:

kustomization/tf-controller/25c8a95e is it possible to set or override generated uid 25c8a95e?

makkes commented 5 months ago

I see that ut uses uid of the provider in commit status like this:

kustomization/tf-controller/25c8a95e is it possible to set or override generated uid 25c8a95e?

No.

hirenko-v commented 5 months ago

@makkes So basicalliy there is no way to have some additional data like cluster name in commit check except setting it in Kustomization(or another resource) name, right?

makkes commented 5 months ago

@makkes So basicalliy there is no way to have some additional data like cluster name in commit check except setting it in Kustomization(or another resource) name, right?

https://github.com/fluxcd/notification-controller/issues/589#issuecomment-1659806646

devnev commented 1 month ago

@makkes the summary field is human-readable text, not an identifier. For this issue, it would be preferable to to change the context value of the github status to make it predictable for each cluster so that other CI steps can wait on the status to be set.