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 possibility to add extra cli.toml configuration that won't be overwritten by this module #322

Open
quba42 opened this issue Nov 21, 2023 · 9 comments

Comments

@quba42
Copy link
Contributor

quba42 commented Nov 21, 2023

This module manages both /etc/pulp/cli.toml and /root/.config/pulp/cli.toml.

This is very inconvenient, since I can't put my extra CLI profiles anywhere without foreman-installer overwriting them with each run.

I can store my CLI profiles in a separate file and then use --config <path_to_my_cli_config> with each CLI command, but that is still pretty inconvenient.

Should we really be managing the root user config with Puppet?

Can't we store all the options we need in /etc/pulp/cli.toml?

I am guessing the reason is:

# ls -al ~/.config/pulp/cli.toml 
-rw-------. 1 root root 80 Nov 21 14:53 /root/.config/pulp/cli.toml

But if I actually look at the file, it does not actually store anything terribly secret (just the path to something secret):

# cat ~/.config/pulp/cli.toml
[cli]
cert = "/etc/foreman/client_cert.pem"
key = "/etc/foreman/client_key.pem"

On a side note: Can we have puppet-pulpcore add something like:

# This file is managed by puppet-pulpcore. Any changes may be overwritten with the next foreman-installer run.

To the cli.toml files it manages?
We already had /root/.config/pulp/cli.toml before foreman-installer came along and started overwriting it, which caused some confusion.

@evgeni
Copy link
Member

evgeni commented Nov 21, 2023

If my memory is right, we do it this way as the certs are not readable by other users and this breaks the cli for them if the cert is defined in the global config.

@quba42
Copy link
Contributor Author

quba42 commented Nov 21, 2023

I am guessing "other users" can only really use pulp status, because most other API endpoints require certs to authenticate with the API. Is it useful for users that don't have access to the certs to want to use the CLI?

@evgeni
Copy link
Member

evgeni commented Nov 21, 2023

Can't those have an own pw or cert?

@quba42
Copy link
Contributor Author

quba42 commented Nov 21, 2023

I really don't know what authentication options there are for Pulp. As far as I can tell, Katello installations always use /etc/foreman/client_*.pem and the dev environment uses:

username = "admin"
password = "password"

I also don't know what happens if you provide both username+password, and cert+key.

@evgeni
Copy link
Member

evgeni commented Nov 21, 2023

According to this weird guy who authored the original code, things explode:
#252 (comment)

@quba42
Copy link
Contributor Author

quba42 commented Nov 22, 2023

Ok, I guess that means there are good reasons why things are split up as they are.
While I am not exactly super happy about this, I guess I will just tell people to put additional config in a separate config file, which can then be used doing something like alias pulp='pulp --config <path_to_alternate_config_file>' .

I guess we can close this ticket.

Should I open a separate ticket for my suggestion to have this module add something like:

# This file is managed by puppet-pulpcore. Any changes may be overwritten with the next foreman-installer run.

to the cli.toml files it manages?

@evgeni
Copy link
Member

evgeni commented Nov 22, 2023

To add some context from an out of band chat:

The reason to edit (one of the) setting file(s) is to add additional profiles, so that the CLI on one machine (e.g. the main Katello box) can be used against multiple Pulp endpoints (e.g. all the content proxies).

That can be achieved by adding smth like this to the config:

[cli-proxy1]  # This CLI profile is named "proxy1"
base_url = "https://proxy1.example.com"
verify_ssl = true
dry_run = true
cert = "/etc/foreman/client_cert.pem"  # Use the same cert as present in the default profile
key = "/etc/foreman/client_key.pem"  # Use the same key as present in the default profile

and then using pulp -p proxy1 …

While discussing this, we noticed that pulp-cli actually reads three places by default: https://github.com/pulp/pulp-cli/blob/main/pulpcore/cli/common/config.py#L16-L20, so if we add those additional profiles to ~/.config/pulp/settings.toml, it won't be overridden by the installer and things should work (it's quite a hack tho, as it makes the configuration confusing).

@quba42 is currently testing this

That all said, I think that both adding a "file is managed" warning and a possibility to enrich the "main" config are fair asks and we should see that we try to implement them nevertheless.

@ekohl
Copy link
Member

ekohl commented Nov 22, 2023

It would be nice if pulp-cli had something like a settings.d directory where you can add server configs in separate files.

@quba42
Copy link
Contributor Author

quba42 commented Nov 22, 2023

I can confirm that using ~/.config/pulp/settings.toml works as expected for adding extra CLI profiles (that won't be overwritten by this module).

I opened a draft PR for adding a "file is managed" comment: #323

@quba42 quba42 changed the title Does this module need to manage both default CLI config file locations? Add possibility to add extra cli.toml configuration that won't be overwritten by this module Nov 22, 2023
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

No branches or pull requests

3 participants