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

Fix broken "read only" fields added in v0.11.0. #835

Merged
merged 2 commits into from
Sep 2, 2021

Conversation

mattmoyer
Copy link
Contributor

These fields were changed as a minor hardening attempt when we switched to Distroless, but I bungled the field names and we never noticed because Kapp doesn't apply API validations.

This change fixes the field names so they act as was originally intended. We should also follow up with a change that validates all of our installation manifest in CI.

Thanks to Jeremy Calcamuggio in Kubernetes Slack for raising this issue.

Release note:

Fixed incorrect "readOnly" and "readOnlyRootFilesystem" field usage in Supervisor and Concierge deployment manifests. 

These fields were changed as a minor hardening attempt when we switched to Distroless, but I bungled the field names and we never noticed because Kapp doesn't apply API validations.

This change fixes the field names so they act as was originally intended. We should also follow up with a change that validates all of our installation manifest in CI.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
@mattmoyer mattmoyer added the bug Something isn't working label Sep 2, 2021
@codecov
Copy link

codecov bot commented Sep 2, 2021

Codecov Report

Merging #835 (c7a8c42) into main (b3b3c23) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #835      +/-   ##
==========================================
- Coverage   78.64%   78.62%   -0.03%     
==========================================
  Files         129      129              
  Lines        8945     8945              
==========================================
- Hits         7035     7033       -2     
- Misses       1681     1684       +3     
+ Partials      229      228       -1     
Impacted Files Coverage Δ
...l/localuserauthenticator/localuserauthenticator.go 56.48% <0.00%> (-0.93%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3b3c23...c7a8c42. Read the comment docs.

…o we can be sure that our manifests pass API validation.

We had this for some components, but not the ones that mattered the most.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
@mattmoyer mattmoyer merged commit 6b7a230 into vmware-tanzu:main Sep 2, 2021
@mattmoyer mattmoyer deleted the fix-readonly-fields branch September 3, 2021 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cla-not-required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants