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

When using Secrets, config functionality should be auto enabled #11258

Closed
kenfinnigan opened this issue Aug 6, 2020 · 13 comments
Closed

When using Secrets, config functionality should be auto enabled #11258

kenfinnigan opened this issue Aug 6, 2020 · 13 comments
Labels
area/kubernetes good first issue Good for newcomers kind/enhancement New feature or request
Milestone

Comments

@kenfinnigan
Copy link
Member

Description
Currently, if an application wants to read configuration from Kubernetes Secrets, but not a ConfigMap, the following properties need to be defined:

quarkus.kubernetes-config.enabled=true
quarkus.kubernetes-config.secrets.enabled=true

Which for me, was confusing at first as I'd assumed the first enablement was specific to ConfigMaps.

Implementation ideas
When quarkus.kubernetes-config.secrets.enabled is true, but quarkus.kubernetes-config.enabled is not present, and thus has the default value, then set it to true to simplify the configuration needed.

@gastaldi gastaldi added the good first issue Good for newcomers label Aug 7, 2020
@viveksahu26
Copy link

I am beginner to this issue. Can you please suggest, how to get started..

@geoand
Copy link
Contributor

geoand commented Sep 2, 2020

Fixed in #11748

@Ladicek
Copy link
Contributor

Ladicek commented Sep 8, 2020

This goes directly against my original intent. The quarkus.kubernetes-config.enabled config property controls whether config sources are created based on ConfigMaps and Secrets. That's also how all other extensions work.

The quarkus.kubernetes-config.secrets.enabled config property is not an alias. It was only meant to enable generating additional Kubernetes resources.

@geoand
Copy link
Contributor

geoand commented Sep 8, 2020

Let's decide on this one before 1.8 goes out

@Ladicek
Copy link
Contributor

Ladicek commented Sep 8, 2020

I don't see a reason to be able to enable/disable only-ConfigMaps or only-Secrets -- you control that by setting the list of ConfigMaps and Secrets to read. You just enable/disable reading config sources from the Kubernetes API server.

So the issue is that I probably should have used a different name for quarkus.kubernetes-config.secrets.enabled :-/

@kenfinnigan
Copy link
Member Author

It's not a one or the other, it's being able to activate secrets without knowing you also must enable the entirety of config for the extension

@geoand
Copy link
Contributor

geoand commented Sep 8, 2020

Basically I'm leaning towards what Ken mentions here - with the caveat that the user shouldn't have to explicitly set quarkus.kubernetes-config.enabled=true.

However, as reported in #11968, we should probably not fetch the secrets when quarkus.kubernetes-config.enabled=false was set explicitly.

WDYT?

@Ladicek
Copy link
Contributor

Ladicek commented Sep 8, 2020

There's no such thing as "activate secrets". You just activate Kubernetes config.

The "secrets.enable" thing just makes Quarkus generate additional Kubernetes resources that give the application's service account access to secrets over the Kubernetes API server's API.

Again, bad name of the property, it seems :-(

@kenfinnigan
Copy link
Member Author

kenfinnigan commented Sep 8, 2020

Irrespective of how it's named, if it needs to be turned on to use secrets, and another config property turned on to make that work, that's a bad experience.

It shouldn't require two configuration properties to be turned on to do one thing

@Ladicek
Copy link
Contributor

Ladicek commented Sep 8, 2020

And the reason why we need the property is that we don't know at build time if the user will want to read stuff from secrets. We could generate the resources always, but for some reason I don't recall, we didn't want that.

EDIT: and that's of course only when you use Quarkus-generated Kubernetes YAML files.

@kenfinnigan
Copy link
Member Author

I don't mind if there's a property to generate the secrets, but I don't see why I need to set another property in addition to that one

@geoand
Copy link
Contributor

geoand commented Sep 8, 2020

I think I can come up with something that fits both of your requirements, checking now

@Ladicek
Copy link
Contributor

Ladicek commented Sep 8, 2020

The 2 properties are totally orthogonal, and only appear to not be when you consider Quarkus-generated Kubernetes YAML files to be the only possible way how to obtain Kubernetes YAML files for Quarkus application.

Frankly, we could just always generate the additional Kubernetes resources and let users know that if they don't need them, they can delete them :-) (Or, instead of having a property to generate them, have a property to suppress generating them.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubernetes good first issue Good for newcomers kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants