-
Notifications
You must be signed in to change notification settings - Fork 56
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
proxy-header
enhancement
#227
proxy-header
enhancement
#227
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
proxy-header
enhancement
There is another earlier problem: keycloak_quarkus_proxy_headers (https://github.com/ansible-middleware/keycloak/blob/main/roles/keycloak_quarkus/meta/argument_specs.yml#L329) is defined as default '', but the default is missing from defaults/main.yml, making it essentially a variable. Can you take care of that here? (adding the "" default (preferred), or otherwise dropping it from arg_specs, and handling the undefined case) |
I do not really understand why the idempotency issue happens here (and not in previous builds); I think we have to drop the changed_when: true from here (which i added about a week ago to have orange logs instead of red, and to silence the linter complaining about ignore_errors...) |
- keycloak_quarkus_proxy_mode is defined | ||
- keycloak_quarkus_proxy_headers is defined and keycloak_quarkus_proxy_headers | length == 0 | ||
- keycloak_quarkus_version.split('.') | first | int >= 24 | ||
changed_when: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose, for the sake of idempotency in tests, to change this to:
changed_when: keycloak_quarkus_deprecation_warnings
keycloak_quarkus_deprecation_warnings is default true, but we set it to false in all molecule tests
wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so now I know that I know nothing; no fuck*** clue why it complains here https://github.com/world-direct/ansible-keycloak/blob/62cbaa35966f7edfeeeb4f76497a4b87de840f7e/roles/keycloak_quarkus/tasks/deprecations.yml#L46 but here it is fine: https://github.com/world-direct/ansible-keycloak/blob/62cbaa35966f7edfeeeb4f76497a4b87de840f7e/roles/keycloak_quarkus/tasks/deprecations.yml#L13
and I've tried plenty...
anyway, I ended up what I should have done from the beginning: following your advice, the new param is called keycloak_quarkus_show_deprecation_warnings
, so let's be done with this PR
e4c7996
to
dd7f85f
Compare
Deprecated
proxy
setting will only be emitted in keycloak.conf ifkeycloak_quarkus_proxy_headers
is not set.Fix #226