Fix NGINX Plus license removal option (#340)

CHANGELOG.md

@@ -7,12 +7,16 @@ ENHANCEMENTS:
 *   Add survey to README.
 *   Improve README structure and use tables where relevant.
 *   Update Ansible (now Ansible base) to `2.10.2`, Ansible (now Ansible Community Distribution) to `2.10.0`, and yamllint to `1.25.0`.
+*   Optimize NGINX Plus install/remove tasks.
 
 BUG FIXES:
 
 *   Prevent TravisCI from trying to build (and failing) NGINX Plus images on external PRs.
-*   Fix naming for SELinux facts dict.
+*   Fix naming for SELinux facts dictionary.
 *   Role now runs correctly when using Ansible's check mode.
+*   Removing the NGINX Plus license in RHEL based distros should no longer return a repository not found error.
+*   Fix issue when removing NGINX Plus license on some distributions.
+*   Fix Amazon Linux NGINX Plus install while at it.
 
 ## 0.17.2 (September 24, 2020)
 

tasks/main.yml

@@ -38,7 +38,7 @@
           tags: nginx_setup_license
 
         - name: Install NGINX Plus
-          include_tasks: "{{ role_path }}/tasks/plus/install-plus.yml"
+          include_tasks: "{{ role_path }}/tasks/plus/install-{{ ansible_facts['os_family'] | lower }}.yml"
           when: nginx_type == "plus"
           tags: nginx_install_plus
 

tasks/plus/install-alpine.yml

@@ -1,15 +1,16 @@
 ---
-- name: (Alpine Linux) Configure NGINX Plus repository
+- name: (Alpine Linux) {{ nginx_license_status is defined | ternary('Remove', 'Configure') }} NGINX Plus repository
   lineinfile:
     path: /etc/apk/repositories
     insertafter: EOF
-    line: "{{ repository }}"
+    line: "{{ nginx_repository | default(nginx_plus_default_repository_alpine) }}"
     state: "{{ nginx_license_status | default ('present') }}"
 
 - name: (Alpine Linux) Install NGINX Plus
   apk:
     name: "nginx-plus{{ nginx_version | default('') }}"
-    repository: "{{ repository }}"
+    repository: "{{ nginx_repository | default(nginx_plus_default_repository_alpine) }}"
     state: "{{ nginx_state }}"
   ignore_errors: "{{ ansible_check_mode }}"
+  when: nginx_license_status is not defined
   notify: (Handler) Run NGINX

tasks/plus/install-debian.yml

@@ -1,5 +1,5 @@
 ---
-- name: (Debian/Ubuntu) Set up NGINX Plus license verification
+- name: (Debian/Ubuntu) {{ nginx_license_status is defined | ternary('Remove', 'Configure') }} NGINX Plus license verification
   blockinfile:
     path: /etc/apt/apt.conf.d/90nginx
     create: yes
@@ -11,10 +11,10 @@
     state: "{{ nginx_license_status | default ('present') }}"
     mode: 0444
 
-- name: (Debian/Ubuntu) Configure NGINX Plus repository
+- name: (Debian/Ubuntu) {{ nginx_license_status is defined | ternary('Remove', 'Configure') }} NGINX Plus repository
   apt_repository:
     filename: nginx-plus
-    repo: "{{ repository }}"
+    repo: "{{ nginx_repository | default(nginx_plus_default_repository_debian) }}"
     update_cache: no
     state: "{{ nginx_license_status | default ('present') }}"
     mode: 0644
@@ -25,4 +25,5 @@
     state: "{{ nginx_state }}"
     update_cache: yes
   ignore_errors: "{{ ansible_check_mode }}"
+  when: nginx_license_status is not defined
   notify: (Handler) Run NGINX

tasks/plus/install-freebsd.yml

@@ -1,5 +1,5 @@
 ---
-- name: (FreeBSD) Set up NGINX Plus license verification
+- name: (FreeBSD) {{ nginx_license_status is defined | ternary('Remove', 'Configure') }} NGINX Plus license verification
   blockinfile:
     path: /usr/local/etc/pkg.conf
     block: |
@@ -8,13 +8,13 @@
       SSL_CLIENT_KEY_FILE: "/etc/ssl/nginx/nginx-repo.key" }
     state: "{{ nginx_license_status | default ('present') }}"
 
-- name: (FreeBSD) Configure NGINX Plus repository
+- name: (FreeBSD) {{ nginx_license_status is defined | ternary('Remove', 'Configure') }} NGINX Plus repository
   blockinfile:
     path: /etc/pkg/nginx-plus.conf
     create: yes
     block: |
       nginx-plus: {
-      URL: {{ repository }}
+      URL: {{ nginx_repository | default(nginx_plus_default_repository_freebsd) }}
       ENABLED: yes
       MIRROR_TYPE: SRV
       }
@@ -25,4 +25,5 @@
   pkgng:
     name: "nginx-plus{{ nginx_version | default('') }}"
     state: "{{ nginx_state }}"
+  when: nginx_license_status is not defined
   notify: (Handler) Run NGINX

tasks/plus/install-plus.yml

@@ -1,8 +0,0 @@
----
-- name: Set NGINX Plus repository
-  set_fact:
-    repository: "{{ nginx_repository |
-                default(nginx_plus_default_repository[(ansible_facts['distribution'] == 'Amazon') | ternary('amazon', ansible_facts['os_family'] | lower)]) }}"
-
-- name: Install NGINX from repository
-  include_tasks: "{{ role_path }}/tasks/plus/install-{{ ansible_facts['os_family'] | lower }}.yml"

tasks/plus/install-redhat.yml

@@ -1,8 +1,9 @@
 ---
-- name: (Amazon Linux/CentOS/Oracle Linux/RHEL) Configure NGINX Plus repository
+- name: (Amazon Linux/CentOS/Oracle Linux/RHEL) {{ nginx_license_status is defined | ternary('Remove', 'Configure') }} NGINX Plus repository
   yum_repository:
     name: nginx-plus
-    baseurl: "{{ repository }}"
+    baseurl: "{{ nginx_repository |
+              default(lookup('vars', 'nginx_plus_default_repository_' + ((ansible_facts['distribution'] == 'Amazon') | ternary('amazon', 'redhat')))) }}"
     description: NGINX Plus Repository
     sslclientcert: /etc/ssl/nginx/nginx-repo.crt
     sslclientkey: /etc/ssl/nginx/nginx-repo.key
@@ -15,8 +16,7 @@
   yum:
     name: "nginx-plus{{ nginx_version | default('') }}"
     state: "{{ nginx_state }}"
-    disablerepo: "*"
-    enablerepo: "nginx-plus"
     update_cache: yes
   ignore_errors: "{{ ansible_check_mode }}"
+  when: nginx_license_status is not defined
   notify: (Handler) Run NGINX

tasks/plus/install-suse.yml

@@ -4,11 +4,12 @@
     src: /etc/ssl/nginx
     dest: /etc/ssl/nginx/nginx-repo-bundle.crt
     mode: 0444
+  when: nginx_license_status is not defined
 
-- name: (SLES) Configure NGINX Plus repository
+- name: (SLES) {{ nginx_license_status is defined | ternary('Remove', 'Configure') }} NGINX Plus repository
   zypper_repository:
     name: nginx-plus
-    repo: "{{ repository }}"
+    repo: "{{ nginx_repository | default(nginx_plus_default_repository_sles) }}"
     state: "{{ nginx_license_status | default ('present') }}"
 
 - name: (SLES) Install NGINX Plus
@@ -17,4 +18,5 @@
     state: "{{ nginx_state }}"
     update_cache: yes
   ignore_errors: "{{ ansible_check_mode }}"
+  when: nginx_license_status is not defined
   notify: (Handler) Run NGINX

tasks/plus/remove-license.yml

@@ -1,5 +1,5 @@
 ---
-- name: Set NGINX Plus license state
+- name: Set NGINX Plus license state to absent
   set_fact:
     nginx_license_status: absent
 
@@ -18,5 +18,5 @@
     - /etc/apk/cert.pem
   when: ansible_facts['distribution'] == "Alpine"
 
-- name: Delete NGINX Plus repository data
-  include_tasks: "{{ role_path }}/tasks/plus/install-plus.yml"
+- name: Remove NGINX Plus repository data
+  include_tasks: "{{ role_path }}/tasks/plus/install-{{ ansible_facts['os_family'] | lower }}.yml"

vars/main.yml

@@ -33,16 +33,15 @@ nginx_default_repository:
         | ternary('mainline/', '') }}sles/{{ ansible_facts['distribution_major_version'] }}"
 
 # Default NGINX Plus repositories
-nginx_plus_default_repository:
-  alpine: "https://plus-pkgs.nginx.com/alpine/v{{ ansible_facts['distribution_version'] | regex_search('^[0-9]+\\.[0-9]+') }}/main"
-  amazon: "https://plus-pkgs.nginx.com/amzn{{ (ansible_facts['distribution_version'] is version('2', '==')) | ternary('2', '') }}/$releasever/$basearch"
-  debian: "deb [arch=amd64] https://plus-pkgs.nginx.com/{{ ansible_facts['distribution'] | lower }} {{ ansible_facts['distribution_release'] }} nginx-plus"
-  freebsd: "https://plus-pkgs.nginx.com/freebsd/${ABI}/latest"
-  redhat: "https://plus-pkgs.nginx.com/{{ (ansible_facts['distribution'] == 'CentOS')
-          | ternary('centos', 'rhel') }}/{{ (ansible_facts['distribution_version'] is version('7.4', '>=')
-          and ansible_facts['distribution_version'] is version('8', '<')) | ternary('7.4', ansible_facts['distribution_major_version']) }}/$basearch/"
-  suse: "https://plus-pkgs.nginx.com/sles/{{ ansible_facts['distribution_major_version'] }}\
-        ?ssl_clientcert=/etc/ssl/nginx/nginx-repo-bundle.crt&ssl_verify=host"
+nginx_plus_default_repository_alpine: "https://plus-pkgs.nginx.com/alpine/v{{ ansible_facts['distribution_version'] | regex_search('^[0-9]+\\.[0-9]+') }}/main"
+nginx_plus_default_repository_amazon: "https://plus-pkgs.nginx.com/amzn{{ (ansible_facts['distribution_major_version'] is version('2', '==')) | ternary('2', '') }}/$releasever/$basearch"
+nginx_plus_default_repository_debian: "deb [arch=amd64] https://plus-pkgs.nginx.com/{{ ansible_facts['distribution'] | lower }} {{ ansible_facts['distribution_release'] }} nginx-plus"
+nginx_plus_default_repository_freebsd: "https://plus-pkgs.nginx.com/freebsd/${ABI}/latest"
+nginx_plus_default_repository_redhat: "https://plus-pkgs.nginx.com/{{ (ansible_facts['distribution'] == 'CentOS')
+                                      | ternary('centos', 'rhel') }}/{{ (ansible_facts['distribution_version'] is version('7.4', '>=')
+                                      and ansible_facts['distribution_version'] is version('8', '<')) | ternary('7.4', ansible_facts['distribution_major_version']) }}/$basearch/"
+nginx_plus_default_repository_sles: "https://plus-pkgs.nginx.com/sles/{{ ansible_facts['distribution_major_version'] }}\
+                                    ?ssl_clientcert=/etc/ssl/nginx/nginx-repo-bundle.crt&ssl_verify=peer"
 
 # Alpine dependencies
 nginx_alpine_dependencies: [