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

[kong] add read-only root filesystem option #104

Merged
merged 3 commits into from
Apr 10, 2020
Merged

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Apr 3, 2020

What this PR does / why we need it:

Adds an option to control enforcing read-only root filesystem in the PodSecurityPolicy.

Special notes for your reviewer:

Prior work in helm/charts#19663

  • The issue where Enterprise wrote to root regardless of its prefix will be addressed in 1.5 (see FTI-1163 for internal discussion). Manual testing of upgrades and fresh installs of 1.5 preview builds confirmed that the issue is clear.
  • The second issue with the container image no longer appears to be present. I was not able to reproduce it with either core 1.5, core 2.0, or Enterprise 1.5.
  • The Postgres sub-chart does not play well with PSP automation, and requires creating a PSP independent of the chart (Bitnami does not provide a stock policy). It cannot reuse the ServiceAccount we create for Kong, as it both requires a writeable root filesystem and access to PVCs.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • PR is based off the current tip of the next branch and targets next, not master
  • New or modified sections of values.yaml are documented in the README.md
  • Title of the PR and commit headers start with chart name (e.g. [kong])

Add "podSecurityPolicy.readOnlyRootFilesystem" to values.yaml, which
defaults to true and controls the readOnlyRootFilesystem parameter in
the PodSecurityPolicy.
Copy link
Member

@hbagdi hbagdi left a comment

Choose a reason for hiding this comment

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

I'm all for making this configurable but setting the default to true seems to be unacceptable at this point.

The Postgres sub-chart does not play well with PSP automation, and requires creating a PSP independent of the chart (Bitnami does not provide a stock policy). It cannot reuse the ServiceAccount we create for Kong, as it both requires a writeable root filesystem and access to PVCs.

We don't need to worry about Postgres sub-chart. Our stand should be to bring your own db when it comes to using this chart whenever anyone wants to do anything more than what the existing chart provides.

@@ -403,6 +403,9 @@ podDisruptionBudget:

podSecurityPolicy:
enabled: false
# Make the root filesystem read-only. This is not compatible with Kong Enterprise <1.5.
# If you use Kong Enterprise <1.5, this must be set to false.
readOnlyRootFilesystem: true
Copy link
Member

Choose a reason for hiding this comment

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

Since this is not compatible with 1.3, we can't put this in as a default.
1.5 is not even GA at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree that we should hold this until 1.5 is out, but are we concerned with having it be the default then? It still only applies to users that have specifically enabled PSPs.

Worst case users already have it enabled and don't read the upgrade guide. Their GUI will be broken because it can't write its config, while the proxy and admin API will function normally, so fairly minor impact.

Move the PodSecurityPolicy spec out of its template into values.yaml.
This allows user control over all PSP settings.
@hbagdi
Copy link
Member

hbagdi commented Apr 7, 2020

Discussed offline with Travis that the blast radius here is very small and we can take a chance with RO Root FS to be true.

@rainest rainest merged commit 2fb842a into next Apr 10, 2020
@hbagdi hbagdi deleted the feat/readonly-root branch April 10, 2020 20:52
rainest added a commit that referenced this pull request Apr 10, 2020
* Move the PodSecurityPolicy spec out of its template into values.yaml.
  This allows user control over all PSP settings.
* Add "podSecurityPolicy.spec.readOnlyRootFilesystem: true" to
  values.yaml
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.

None yet

2 participants