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

Add support for enabling/disabling the replication of object types #335

Conversation

aljoshare
Copy link
Contributor

This PR adds the support for enabling or disabling the replication of object types and strengthens the principle of least privilege. For example, you can now disable the replication of roles, role bindings or service accounts if thats not needed for your use case.

Fixes #284

@aljoshare
Copy link
Contributor Author

@martin-helmich For now, I added the flags as examples to the args in the helm values. I could also introduce separate values and toggle also the RBAC (as you wrote in the issue). Should I do that or leave it like this? 😊

martin-helmich
martin-helmich previously approved these changes Apr 4, 2024
Copy link
Member

@martin-helmich martin-helmich left a comment

Choose a reason for hiding this comment

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

Hey @aljoshare! Thanks for the contribution! 👍

I could also introduce separate values and toggle also the RBAC (as you wrote in the issue). Should I do that or leave it like this? 😊

I think for a least-privilege implementation, it would absolutely make sense to also tailor the RBAC roles on the desired replication targets. We can leave this PR as-is, and add that in a follow-up PR (in which case I'd be happy merging this), or adjust this PR -- whatever you prefer. 🙂

@aljoshare
Copy link
Contributor Author

@martin-helmich I added a separate config for the toggles, added them directly to the deployment and refactored the RBAC a little to make it more readable with flow control.

There is one thing which is maybe not backward compatible: If someone copied the args: [] instead of just relying on the default value, it would break now. Do you think we should handle this case by adding more complexity or leave it like this because it's not a good idea to copy defaults anyway?

@aljoshare
Copy link
Contributor Author

@martin-helmich Ping 😇

Copy link
Member

@martin-helmich martin-helmich left a comment

Choose a reason for hiding this comment

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

Agh, apologies. 😱🙏 Looks good to me now. 👍

@martin-helmich martin-helmich enabled auto-merge (squash) April 12, 2024 07:34
@martin-helmich martin-helmich merged commit f2cc629 into mittwald:master Apr 12, 2024
3 checks passed
@martin-helmich
Copy link
Member

If someone copied the args: [] instead of just relying on the default value, it would break now. Do you think we should handle this case by adding more complexity or leave it like this because it's not a good idea to copy defaults anyway?

I'm not sure if this is actually breaking; IIRC, with should not evaluate to true with an empty slice. In any case, I think the probability that a user would have explicitly overridden args: [] is slim. I'll keep an ear open if any feedback comes in on this.

@aljoshare
Copy link
Contributor Author

Agh, apologies. 😱🙏 Looks good to me now. 👍

No problem 😊 Thank you!

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.

Configurable object types only being replicated
2 participants