Skip to content
This repository has been archived by the owner on Jan 30, 2024. It is now read-only.

Import or generate key for token authentication #361

Merged
merged 1 commit into from
Aug 7, 2020
Merged

Import or generate key for token authentication #361

merged 1 commit into from
Aug 7, 2020

Conversation

mdellweg
Copy link
Member

@pulpbot
Copy link
Member

pulpbot commented Jul 14, 2020

Attached issue: https://pulp.plan.io/issues/7098

@mdellweg mdellweg force-pushed the install_token_keys branch 2 times, most recently from 6823c8a to 1b9455e Compare July 14, 2020 14:10
@mdellweg
Copy link
Member Author

mdellweg commented Jul 14, 2020 •
edited
Loading

Missing:

  • Configuring it.

mikedep333
mikedep333 reviewed Jul 17, 2020
View reviewed changes
Copy link
Member

@mikedep333 mikedep333 left a comment

Choose a reason for hiding this comment

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

I have to ask about the design.

We've alternated between using paths like /etc/pulp/private_key.pem and /var/lib/pulp/cert/private.pem

Did we agree on what it should be? I remember a discussion.

@mdellweg
Copy link
Member Author

I have to ask about the design.

We've alternated between using paths like /etc/pulp/private_key.pem and /var/lib/pulp/cert/private.pem

Did we agree on what it should be? I remember a discussion.

Yes, there was a discussion, but no result that i can remember. I think /etc/pki/pulp/*.pem was another option.

@mdellweg mdellweg force-pushed the install_token_keys branch 2 times, most recently from e009aee to 0ee07e0 Compare July 20, 2020 10:40
@mdellweg
Copy link
Member Author

I just figured, that the failures here are probably due to the openssl-modules have changed their library requirement from 2.8 to 2.9.
So glad we tests different ansible versions now...

@mdellweg mdellweg force-pushed the install_token_keys branch 3 times, most recently from 3395126 to ed5143d Compare July 21, 2020 10:58
@mdellweg
Copy link
Member Author

mdellweg commented Jul 21, 2020 •
edited
Loading

The last combination still not working is Centos-7 wiht ansible 2.8, because the version of pyOpenSSL in Centos-7 is too old.
Worked around it with the command module.

@dkliban
Copy link
Member

dkliban commented Jul 22, 2020

Let's put these in /etc/pulp/

@mdellweg mdellweg force-pushed the install_token_keys branch 2 times, most recently from 764d3fa to 9a9fda3 Compare July 23, 2020 13:46
@mdellweg mdellweg mentioned this pull request Jul 24, 2020
Merged
@mdellweg mdellweg force-pushed the install_token_keys branch 2 times, most recently from b3b82cc to 3c49f8c Compare July 24, 2020 11:02
dkliban
dkliban reviewed Jul 30, 2020
View reviewed changes
roles/pulp_common/vars/main.yml Outdated
@@ -10,6 +10,7 @@ pulp_install_plugins_normalized_yml: |-
# A pulp_install_plugins but with the plugin names corrected:
# pip/PyPI only uses dashes, not underscores.
pulp_install_plugins_normalized: "{{ pulp_install_plugins_normalized_yml | from_yaml }}"
__pulp_common_pulp_pki_dir: "{{ pulp_config_dir }}/cert"
Copy link
Member

Choose a reason for hiding this comment

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

let's make cert plural. "{{ pulp_config_dir }}/certs"

@mdellweg mdellweg force-pushed the install_token_keys branch 3 times, most recently from 32916fb to 5ecf7f5 Compare August 3, 2020 09:16
mikedep333
mikedep333 approved these changes Aug 3, 2020
View reviewed changes
Copy link
Member

@mikedep333 mikedep333 left a comment

Choose a reason for hiding this comment

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

To be honest, I am surprised CI is passing. See comments in particular about become.

roles/pulp_api/tasks/main.yml Show resolved Hide resolved
roles/pulp_api/tasks/generate_token_auth_key.yml Outdated Show resolved Hide resolved
roles/pulp_api/tasks/import_token_auth_key.yml Outdated Show resolved Hide resolved
roles/pulp_api/tasks/main.yml Outdated Show resolved Hide resolved
roles/pulp_api/README.md Show resolved Hide resolved
roles/pulp_common/vars/CentOS-7.yml Outdated
@@ -9,3 +9,5 @@ pulp_preq_packages:
- gcc # For psycopg2
- make # For make docs
pulp_python_interpreter: /usr/bin/python3.6
pulp_common_python_cryptography:
Copy link
Member

Choose a reason for hiding this comment

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

I cannot find any collection-specific guidelines on this (or on our use case of having a common role), but I think we should name this pulp_python_cryptography.

The reason being that "common" is implied in the generic "pulp" prefix, and it's less verbose.

It might also be moved to another role later, if only that role needs it.

roles/pulp_api/tasks/main.yml Show resolved Hide resolved
mikedep333
mikedep333 suggested changes Aug 3, 2020
View reviewed changes
Copy link
Member

@mikedep333 mikedep333 left a comment

Choose a reason for hiding this comment

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

My previous review was accidentally "approve". The only mandatory comments about the become / become_user. If they are not necessary for some reason, I would really like to know. I suspect they are necessary, but the molecule / docker CI env always connects as root.

@dkliban
Copy link
Member

dkliban commented Aug 5, 2020

Change the file names to include 'token' in the name.

@dkliban dkliban merged commit af31b1d into pulp:master Aug 7, 2020
@mdellweg mdellweg deleted the install_token_keys branch August 8, 2020 10:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dkliban dkliban dkliban approved these changes

@mikedep333 mikedep333 mikedep333 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mdellweg @pulpbot @dkliban @mikedep333