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

Custom Git Commit Status Prefix #209

Closed
wants to merge 2 commits into from
Closed

Custom Git Commit Status Prefix #209

wants to merge 2 commits into from

Conversation

clarkdoan
Copy link

Resolves: #194

Problem

When setting a git commit status the controller only uses the event.InvolvedObject.Kind and event.InvolvedObject.Name to generate the status' context. This does not allow multiple clusters to update the commit status if they use the same Kustomization name.

Proposed Solution

The event that is passed to the formatNameAndDescription function contains a Metadata field.

The AlertSpec contains a Summary field which is used as a Short description of the impact and affected cluster.

The event handler copies the value of the Summary field from the AlertSpec to the Metadata field of the event.

This PR checks the value of the Metadata field to see if there is a summary key. If the key exists then the summary value will be prepended to the context of the commit status. This will allow the user to set the prefix of the commit status.

Signed-off-by: Clark Doan <18647423+clarkdoan@users.noreply.github.com>
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the Summary description in code and run make test then commit the generated CRDs. Append this to the field description: "For commit status updates, this field is used to prefix the status message.".

@phillebaba
Copy link
Member

We need to do some testing with this feature regarding different provider length constraints and format constraints, as this will allow users to inject arbitrary text into the status ID.

@stefanprodan
Copy link
Member

@phillebaba I think we should trim the summary to a small number of characters, as you said we need to determine which number that is.

@phillebaba
Copy link
Member

Github has a limit of 140 last time I hit this issue but is not documented. Gitlab has something similar while Azure DevOps has a much larger limit. The issue is that none of these limits are documented.

@clarkdoan clarkdoan requested a review from stefanprodan June 14, 2021 19:30
@phillebaba
Copy link
Member

I would actually prefer to hold off on this PR and instead implement a more generic solution as proposed here.
fluxcd/flux2#1529

@clarkdoan clarkdoan closed this by deleting the head repository Oct 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding cluster name (or something else) to Github commit status context
3 participants