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

feat: DeletionPolicy support for webhook provider (external-secrets#1… #2066

Merged

Conversation

ArtificialQualia
Copy link
Contributor

Problem Statement

As identified in #1958 the webhook provider needs to be updated to return the correct error so that it may properly support the Merge/Delete DeletionPolicy.

Related Issue

Fixes #1958

Proposed Changes

This PR updates the webhook provider to return the appropriate error type on receiving a 404 from a webhook so that the DeletionPolicy logic will trigger correctly.

Checklist

  • I have read the contribution guidelines
  • All commits are signed with git commit --signoff
  • My changes have reasonable test coverage
  • All tests pass with make test
  • I ensured my PR is ready for review with make reviewable

)

Signed-off-by: ArtificialQualia <kendall.masse@gmail.com>
@ArtificialQualia ArtificialQualia requested a review from a team as a code owner February 27, 2023 18:32
@ArtificialQualia ArtificialQualia requested review from moolen and removed request for a team February 27, 2023 18:32
Copy link
Contributor

@paul-the-alien paul-the-alien bot left a comment

Choose a reason for hiding this comment

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

Greetings!
Thank you for contributing to this project!
If this is your first time contributing, please make
sure to read the Developer and Contributing Process guides.
Please also mind and follow our Code of Conduct.

Useful commands:

  • make fmt: Formats the code
  • make check-diff: Ensures the branch is clean
  • make reviewable: Ensures a PR is ready for review

Copy link
Member

@moolen moolen left a comment

Choose a reason for hiding this comment

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

Looks good, could you please add a note to the provider docs that a 404 may cause secret deletion? 🙏

@sonarcloud
Copy link

sonarcloud bot commented Feb 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ArtificialQualia
Copy link
Contributor Author

Looks good, could you please add a note to the provider docs that a 404 may cause secret deletion? 🙏

This has now been added

Copy link
Member

@moolen moolen left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this 🙇

@moolen moolen merged commit 44bb3c4 into external-secrets:main Feb 27, 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.

DeletionPolicy support for the Webhook provider
2 participants