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 custom extra configmaps/secrets #208

Merged
merged 3 commits into from
Oct 19, 2020
Merged

Conversation

mikemenaker
Copy link
Contributor

@mikemenaker mikemenaker commented Oct 5, 2020

Add extraConfigMaps and extraSecrets fields to values file that will be added to volumes/volumeMounts template helpers.

What this PR does / why we need it:

Adds ability to mount arbitrary config maps/secrets

Which issue this PR fixes

closes #196

Special notes for your reviewer:

Checklist

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

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

Add extraConfigMaps and extraSecrets fields to values file that will be added to volumes/volumeMounts template helpers.
@CLAassistant
Copy link

CLAassistant commented Oct 5, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@rainest rainest left a comment

Choose a reason for hiding this comment

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

Woo, finally got a chance to play around with this after Kong Summit stuff. We had some history issues with our release process as part of that, so you'll want to merge in the latest next tip.

Generally looks good as far as mounting stuff, but I have one change request and one question that's probably also a change request. Simpler one first!

For the comments, I'd suggest making the inline comments a double-comment with some brief explanation, so that removing the leading # doesn't include anything that will parse as YAML other than the the example string itself, e.g.

#   subPath: my-subpath # Optional, if you wish to mount a single key and not the entire ConfigMap

instead of:

#   subPath: my-subpath [OPTIONAL]

More complex one second: do you know of any reason to include the readOnly field? That looks like maybe some confusion introduced by the K8S docs and volume mount spec, which address all types of volume mounts equally. As best I can tell, R/W ConfigMap or Secret mounts aren't actually allowed, so I'd say remove that setting (and the associated template content) unless you know of some edge case that'd require it still.

ConfigMap/Secret mounts appear to be read-only always, and K8S will just ignore silently ignore that setting for them. Per the other issue comments, this behavior is itself a source of confusion, but apparently exists as a legacy holdover from when K8S did allow you to modify mounted ConfigMaps and Secrets.

@mikemenaker
Copy link
Contributor Author

I updated the fork with the requested changes, I'll merge/rebase the last 'next' branch tomorrow morning.

@mikemenaker
Copy link
Contributor Author

@rainest updated comments, removed readOnly, merged in latest next branch

@rainest
Copy link
Contributor

rainest commented Oct 16, 2020

Bizarre. CI keeps failing because a Postgres-backed instance cannot connect to Postgres, but I cannot replicate this locally with the same test values on a fresh install/namespace.

Possibly something awry with GH actions at the moment; will try tests again on Monday.

@rainest rainest merged commit 79aeb8b into Kong:next Oct 19, 2020
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

3 participants