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

rewrite option is not created with default template #192

Closed
tcraxs opened this issue Oct 27, 2021 · 3 comments
Closed

rewrite option is not created with default template #192

tcraxs opened this issue Oct 27, 2021 · 3 comments
Labels
bug Something isn't working
Milestone

Comments

@tcraxs
Copy link
Contributor

tcraxs commented Oct 27, 2021

Describe the bug

In the default scenario in molecule test is (see here) a rewrites option configured. But in the latest molecule output on main branch, you can not find this entry in the generated /etc/nginx/conf.d/default.conf (see here).

I also tested this locally and in all my tests (with molecule), I did not get it working to have a rewrite option in the config.

To reproduce

Run locally molecule test -s default

Expected behavior

Having a rewrite entry in the generated config, e.g. in server context:

server {
  ...
  rewrite (.*).html(.*) $1$2 last;
  ...
}

Your environment

Test with debian-buster (docker) molecule the default scenario.

Additional context

I guess the changes in #189 are not covering all sequences. After changing this line

{% for rewrite in rewrite['rewrites'] if (rewrite['rewrites'] is sequence and rewrite['rewrites'] is not string) %}

to that:

{% for rewrite in rewrite['rewrites'] if (rewrite['rewrites'] is not mapping and rewrite['rewrites'] is not string) %}

the molecule test (I tested it only on debian-buster) was still good and output of the task [ansible-role-nginx-config : Print NGINX config] shows me the rewrite option in the config.

@tcraxs
Copy link
Contributor Author

tcraxs commented Oct 28, 2021

Created a PR to solve this issue. Also this Molecule test is added and I test it before my change (and of course it was failing). Add this to the verify.yml:

    - name: Ensure default.conf contains 'rewrite (.*).html(.*) $1$2 last;'
      lineinfile:
        path: /etc/nginx/conf.d/default.conf
        line: "    rewrite (.*).html(.*) $1$2 last;"
        state: present
      check_mode: true
      register: conf
      failed_when: (conf is changed) or (conf is failed)

@alessfg alessfg added the bug Something isn't working label Oct 28, 2021
@alessfg alessfg added this to To do in NGINX Configuration via automation Oct 28, 2021
@alessfg alessfg added this to the 0.4.2 milestone Oct 28, 2021
@alessfg
Copy link
Collaborator

alessfg commented Oct 28, 2021

Looks like I missed some sequences indeed! I'll have a different PR ready shortly to address the remaining sequences I missed.

@tcraxs
Copy link
Contributor Author

tcraxs commented Oct 28, 2021

Thanks @alessfg for fast ack and merging. If you need a second pair of eyes, please ping me :)

@tcraxs tcraxs closed this as completed Oct 28, 2021
NGINX Configuration automation moved this from To do to Done Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

No branches or pull requests

2 participants