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

Feature/last updated by annotation #138

Merged

Conversation

arielsepton
Copy link
Contributor

@arielsepton arielsepton commented Apr 16, 2024

This PR adds a mutating webhook to manage the rcs.dana.io/last-updated-by annotation on Capp objects, tracking user-initiated changes.
The mutating webhook only performs the annotation adding for CREATE and UPDATE operations. DELETE operations are allowed without modification since mutation webhooks do not perform any mutations on delete operations.

References:

@dana-prow-ci dana-prow-ci bot requested a review from mzeevi April 16, 2024 15:28
Copy link
Contributor

@mzeevi mzeevi left a comment

Choose a reason for hiding this comment

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

Overall looks great, added a few comments. Also please squash the commits and only push a single commit. We try to adhere to similar standards to this

internal/webhooks/capp_mutator_webhook.go Outdated Show resolved Hide resolved
internal/webhooks/capp_mutator_webhook.go Outdated Show resolved Hide resolved
test/e2e_tests/mutating.go Outdated Show resolved Hide resolved
test/e2e_tests/mutating.go Outdated Show resolved Hide resolved
test/e2e_tests/mutating.go Outdated Show resolved Hide resolved
test/e2e_tests/mutating.go Outdated Show resolved Hide resolved
test/e2e_tests/utils/testuser_adapter.go Show resolved Hide resolved
test/e2e_tests/utils/testuser_adapter.go Show resolved Hide resolved
test/e2e_tests/suite_test.go Show resolved Hide resolved
Copy link
Contributor

@mzeevi mzeevi left a comment

Choose a reason for hiding this comment

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

Added a few more comments

internal/webhooks/capp_mutator_webhook.go Show resolved Hide resolved
internal/webhooks/capp_mutator_webhook.go Outdated Show resolved Hide resolved
internal/webhooks/capp_mutator_webhook.go Outdated Show resolved Hide resolved
test/e2e_tests/utils/testuser_adapter.go Outdated Show resolved Hide resolved
test/e2e_tests/utils/testuser_adapter.go Outdated Show resolved Hide resolved
test/e2e_tests/utils/testuser_adapter.go Outdated Show resolved Hide resolved
test/e2e_tests/utils/capp_adpater.go Outdated Show resolved Hide resolved
…ds/modifies a last-updated-by annotation on every creation/update of Capp

Signed-off-by: Ariel Septon <64063409+arielsepton@users.noreply.github.com>
@arielsepton arielsepton force-pushed the feature/last-updated-by-annotation branch from 2ca6a6a to 67e2817 Compare April 18, 2024 10:51
Copy link
Contributor

@mzeevi mzeevi left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Copy link
Contributor

dana-prow-ci bot commented Apr 18, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arielsepton, mzeevi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dana-prow-ci dana-prow-ci bot merged commit a95b1a2 into dana-team:main Apr 18, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants