Skip to content

Use repository modules instead of copy #16

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Use repository modules instead of copy #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aeb0bea
Use the `apt_repository` module on Debian systems
mcdonnnj Apr 29, 2025
9e5a813
Use the `yum_repository` module on RedHat systems
mcdonnnj Apr 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions tasks/setup_Debian.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@
executable: /bin/bash

- name: Add the Okta ASA repo
ansible.builtin.copy:
content: >-
ansible.builtin.apt_repository:
Copy link
Member

Choose a reason for hiding this comment

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

Ideally we would use ansible.builtin.deb822_repository where possible. See, for example, here.

Copy link
Member Author

@mcdonnnj mcdonnnj Apr 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I was planning to add that functionality in a follow-up PR though I was going to follow the logic in cisagov/ansible-role-backports.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with this happening in a follow-on PR.

The nice thing about the example I provided is that it only checks for the presence of Buster. That means the logic will still work as written as new Debian versions are released. Or is there another reason to prefer the logic in the backports role?

Copy link
Member Author

Choose a reason for hiding this comment

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

No reason to prefer it, I would just prefer we be consistent. I knew backports role did a similar split-config with some getting DEB822 and some getting classic which is why I was using it as a reference. I was more just mentioning that my original intent was to use that logic and not arguing in favor of that logic.

filename: oktapam-stable
repo: >-
deb [signed-by=/usr/share/keyrings/oktapam-2023-archive-keyring.gpg]
https://dist.scaleft.com/repos/deb {{ ansible_distribution_release }}
okta
dest: /etc/apt/sources.list.d/oktapam-stable.list
mode: u=rw,g=,o=
20 changes: 8 additions & 12 deletions tasks/setup_RedHat.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,17 @@
ternary(ansible_distribution + 'Linux', ansible_distribution) }}
- name: Add the Okta ASA repo
ansible.builtin.copy:
# yamllint complains about the length of a few of the lines below,
ansible.builtin.yum_repository:
# yamllint complains about the length of a couple of the lines below,
# but there is no way to shorten them.
#
# yamllint disable rule:line-length
Comment on lines +20 to 24
Copy link
Member

Choose a reason for hiding this comment

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

I think you can remove the yamllint disable directive...

Suggested change
ansible.builtin.yum_repository:
# yamllint complains about the length of a couple of the lines below,
# but there is no way to shorten them.
#
# yamllint disable rule:line-length
ansible.builtin.yum_repository:

content: |
[oktapam-stable]
name=Okta PAM Stable - {{ ansible_distribution_tweaked }} {{ ansible_distribution_major_version }}
baseurl=https://dist.scaleft.com/repos/rpm/stable/{{ ansible_distribution_tweaked | lower }}/{{ ansible_distribution_major_version }}/$basearch
gpgcheck=1
repo_gpgcheck=1
enabled=1
gpgkey=https://dist.scaleft.com/GPG-KEY-OktaPAM-2023
baseurl: https://dist.scaleft.com/repos/rpm/stable/{{ ansible_distribution_tweaked | lower }}/{{ ansible_distribution_major_version }}/$basearch
description: Okta PAM Stable - {{ ansible_distribution_tweaked }} {{ ansible_distribution_major_version }}
Comment on lines +25 to +26
Copy link
Member

@jsf9k jsf9k Apr 29, 2025 •
edited by mcdonnnj
Loading

Choose a reason for hiding this comment

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

...if you make these changes:

Suggested change
baseurl: https://dist.scaleft.com/repos/rpm/stable/{{ ansible_distribution_tweaked | lower }}/{{ ansible_distribution_major_version }}/$basearch
description: Okta PAM Stable - {{ ansible_distribution_tweaked }} {{ ansible_distribution_major_version }}
baseurl: >-
https://dist.scaleft.com/repos/rpm/stable/{{
ansible_distribution_tweaked | lower }}/{{
ansible_distribution_major_version }}/$basearch
description: >-
Okta PAM Stable - {{ ansible_distribution_tweaked }}
{{ ansible_distribution_major_version }}

Copy link
Member Author

Choose a reason for hiding this comment

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

Thoughts on changing the description to:

Okta PAM Stable - {{ ansible_distribution_tweaked }}
{{ ansible_distribution_major_version }}

Since that still maintains the desired format without having to chop into the substitution.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's fine.

# Re-enable the line-length yamllint rule.
#
# yamllint enable rule:line-length
Comment on lines 27 to 29
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed too:

Suggested change
# Re-enable the line-length yamllint rule.
#
# yamllint enable rule:line-length

dest: /etc/yum.repos.d/oktapam-stable.repo
mode: u=rw,g=,o=
gpgcheck: true
gpgkey: https://dist.scaleft.com/GPG-KEY-OktaPAM-2023
name: oktapam-stable
repo_gpgcheck: true