Use repository modules instead of copy

[mcdonnnj]

🗣 Description

This pull request replaces the use of the copy module with apt_repository for Debian-based systems and yum_repository for RedHat-based systems.

💭 Motivation and context

Using these modules avoid having to manually configure file paths and permissions appropriately and results in a better maintenance position for this role.

🧪 Testing

Automated tests pass.

✅ Pre-approval checklist

• This PR has an informative and human-readable title.
• Changes are limited to a single goal - eschew scope creep!
• All relevant type-of-change labels have been added.
• I have read the CONTRIBUTING document.
• These code changes follow cisagov code standards.
• All new and existing tests pass.

[Copilot]

Pull Request Overview

This pull request replaces the use of the ansible.builtin.copy module with repository-specific modules to manage package repositories more cleanly on Debian-based and RedHat-based systems.

• For RedHat systems, the copy module was replaced with ansible.builtin.yum_repository, and repository configuration options were updated accordingly.
• For Debian systems, the copy module was replaced with ansible.builtin.apt_repository with appropriate repository parameters.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
tasks/setup_RedHat.yml	Replaced the use of the copy module with yum_repository and adjusted options.
tasks/setup_Debian.yml	Replaced the use of the copy module with apt_repository and updated parameters.

[jsf9k]

I made some suggestions, which you should feel free to argue against if you disagree with them.

[jsf9k]

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:

[jsf9k]

...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 }}

[mcdonnnj]

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.

[jsf9k]

Yes, that's fine.

[jsf9k]

This can be removed too:

Suggested change

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

[jsf9k]

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

[mcdonnnj]

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.

[jsf9k]

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?

[mcdonnnj]

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.