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

[ansible/artifactory] refactored Nginx role (DRY). #337

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

Conversation

bbaassssiiee
Copy link
Contributor

PR Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Title of the PR starts with installer/product name (e.g. [ansible/artifactory])
  • CHANGELOG.md updated
  • Variables and other changes are documented in the README.md

What this PR does / why we need it:

There were two roles in master that basically did the same thing and duplicated a lot of code:

  • artifactory_nginx
  • artifactory_nginx_ssl

I rewrote artifactory_nginx so it behaves just like the other two (which were mutually exclusive in use). Also I added a way to install the Nginx module 1.20 (or newer) from the Yum repo's configured on the machine. This is helpful in on-premises deployments where every proxy whitelist item needs permission from security.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #
Partial fix for #323

Special notes for your reviewer:

@bbaassssiiee bbaassssiiee marked this pull request as draft October 2, 2023 21:30
@bbaassssiiee bbaassssiiee marked this pull request as draft October 2, 2023 21:30
@Logeshwarsn
Copy link
Contributor

@bbaassssiiee Thanks for the PR. Its look like a big change, we would like to review this internally and get back to you shortly.

@bbaassssiiee
Copy link
Contributor Author

bbaassssiiee commented Oct 10, 2023

@olwe0002 You raised issue #323, maybe you can review this PR? Especially the Debian/Ubuntu part.

@bbaassssiiee bbaassssiiee marked this pull request as ready for review October 10, 2023 08:02
@bbaassssiiee
Copy link
Contributor Author

Fix for #323

@bbaassssiiee
Copy link
Contributor Author

@bbaassssiiee Thanks for the PR. Its look like a big change, we would like to review this internally and get back to you shortly.

Actually this reduces the amount of code to maintain.

@bbaassssiiee bbaassssiiee deleted the feature/323 branch December 11, 2023 09:17
@bbaassssiiee bbaassssiiee restored the feature/323 branch December 11, 2023 09:18
@bbaassssiiee bbaassssiiee reopened this Dec 11, 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

Successfully merging this pull request may close these issues.

2 participants