Skip to content
This repository was archived by the owner on Apr 17, 2025. It is now read-only.

Read-only client in webhooks only mode #206

Merged
merged 1 commit into from
Apr 24, 2022

Conversation

erikgb
Copy link
Contributor

@erikgb erikgb commented Apr 23, 2022

This appears like a more robust implementation of the webhooks only mode than the current one. It ensures that the client provided to all components are not able to perform any write operations towards the API.

Tested: Ran both unit-tests ('make test') and integration test ('make test-e2e') successfully. The integration tests were verified both in default and HA mode.

Fixes #198, fixes #195

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 23, 2022
@erikgb
Copy link
Contributor Author

erikgb commented Apr 23, 2022

/cc @adrianludwin
/cc @rjbez17

@erikgb erikgb force-pushed the feat/readonly-client branch 2 times, most recently from ded0159 to 414ad55 Compare April 23, 2022 20:44
Copy link
Contributor

@adrianludwin adrianludwin left a comment

Choose a reason for hiding this comment

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

Awesome change :) Just a few suggestions

This appears like a more robust implementation of the webhooks only mode than the current one. It ensures that the client provided to all components are not able to perform any write operations towards the API.

Tested: Ran both unit-tests ('make test') and integration test ('make test-e2e') successfully. The integration tests were verified both in default and HA mode.
@erikgb erikgb force-pushed the feat/readonly-client branch from 414ad55 to 683a23b Compare April 24, 2022 17:46
@erikgb
Copy link
Contributor Author

erikgb commented Apr 24, 2022

@adrianludwin I have addressed all your comments. PTAL!

Copy link
Contributor

@adrianludwin adrianludwin left a comment

Choose a reason for hiding this comment

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

I like it! Thanks!

/lgtm
/approve

@adrianludwin adrianludwin added this to the release-v1.1 milestone Apr 24, 2022
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 24, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianludwin, erikgb

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 24, 2022
@k8s-ci-robot k8s-ci-robot merged commit e3937b3 into kubernetes-retired:master Apr 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Read-only mode implemented with read-only client Make HRQ HA-compliant
3 participants