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

Deactivated Grafana reporting in monitoring example yaml. #31989

Merged
merged 1 commit into from
May 2, 2024

Conversation

mvtab
Copy link
Contributor

@mvtab mvtab commented Apr 16, 2024

Change description

Added two configuration parameters to Grafana for disabling most of it's reporting capabilities. Feedback links are let on.

Testing

Tested for 12h on dev kube cluster and no report requests were noticed, while the monitoring example kept working as expected.

### References
Fixes: GHSA-mxv3-27mh-wcfj security advisory

@mvtab mvtab requested a review from a team as a code owner April 16, 2024 11:02
@mvtab mvtab requested a review from tommyp1ckles April 16, 2024 11:02
@maintainer-s-little-helper
Copy link

Commit aa14979 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Apr 16, 2024
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Apr 16, 2024
@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Apr 16, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 16, 2024
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Thanks for the submission, looks good to me. As this is just an example YAML and the configuration has expected behaviour for the upstream project, we will not treat this as a security vulnerability in Cilium.

In order to accept this change, you must agree to the Developer's Certificate of Origin and signal this by adding a Signed-off-by: ... tag to your commits (git commit --amend -s should do the trick). For more details, see the response from the bot above.

@tommyp1ckles
Copy link
Contributor

Changes lgtm, I'll approve following fixing the commit signature

@maintainer-s-little-helper
Copy link

Commits aa14979, 90217ad do not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@mvtab
Copy link
Contributor Author

mvtab commented Apr 16, 2024

I am sorry, did not know about that.
The new commit should be fine now. 959a593

@maintainer-s-little-helper
Copy link

Commit 433b78e does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link

Commit b4c1848 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link

Commit f975b54 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link

Commit 76e8601 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Apr 16, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Apr 16, 2024
@mvtab
Copy link
Contributor Author

mvtab commented Apr 16, 2024

Took me a while to make a verified sign f1a8bac cbfdf2e

@joestringer
Copy link
Member

/test

@maintainer-s-little-helper
Copy link

Commit 51cb772 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Apr 17, 2024
@joestringer
Copy link
Member

Hi @mvtab , there's no need to merge the main branch into the PR each day; Cilium is a fast-moving project and you'll almost always be out-of-date with respect to the main development branch. Perhaps after a week or two it may be useful to do a rebase (not merge) but otherwise I would suggest to hold off on making further changes.

If you can rebase the PR once more against the main branch to drop the merge commit + force push into this branch, then I can go and retrigger the tests again.

@maintainer-s-little-helper
Copy link

Commit c5ede4c does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

Signed-off-by: mvtab <mvtabilitas@protonmail.com>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Apr 17, 2024
@mvtab
Copy link
Contributor Author

mvtab commented Apr 17, 2024

Hi @joestringer, noted. Thanks.
The whole two lines of code should be finally ready now 😅. ceed392

@joestringer
Copy link
Member

/test

@joestringer joestringer added this pull request to the merge queue May 2, 2024
Merged via the queue into cilium:main with commit d4ad5bc May 2, 2024
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/community-contribution This was a contribution made by a community member. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants