Skip to content
This repository has been archived by the owner on Jun 7, 2024. It is now read-only.

#951 Replace AuthMode.OFF by AuthMode.NONE #978

Closed

Conversation

fbrns
Copy link
Member

@fbrns fbrns commented Nov 9, 2018

AuthMode.NONE is added as an addition to AuthMode.OFF

Description

When using application.yml to configure nakadi.oauth2.mode YAML parser translates OFF to false if it isn't put into quotes. Adding a NONE value in the enum that has the same effect as OFF prevents that and OFF can be kept for backward compatibility.

Review

  • Tests
  • Documentation

@codecov-io
Copy link

codecov-io commented Nov 9, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@6aedd56). Click here to learn what that means.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #978   +/-   ##
=========================================
  Coverage          ?   54.16%           
  Complexity        ?     1776           
=========================================
  Files             ?      335           
  Lines             ?     9589           
  Branches          ?      882           
=========================================
  Hits              ?     5194           
  Misses            ?     4074           
  Partials          ?      321
Impacted Files Coverage Δ Complexity Δ
...va/org/zalando/nakadi/security/ClientResolver.java 28.57% <0%> (ø) 3 <0> (?)
...va/org/zalando/nakadi/config/SecuritySettings.java 36.84% <100%> (ø) 0 <0> (?)
...g/zalando/nakadi/config/SecurityConfiguration.java 92.68% <100%> (ø) 11 <0> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6aedd56...1072241. Read the comment docs.

@adyach
Copy link
Member

adyach commented Apr 23, 2019

@fbrns thank you for contribution!
can we just put OFF in quotes? I also see that this behaviour is changed for yaml 1.2, which interprets it as not a boolean. we have never observed such behaviour, most probably, the reason is that we use Nakadi with docker compose.

@fbrns
Copy link
Member Author

fbrns commented Apr 23, 2019

Putting OFF into quotes could a quick but a bit awkward workaround (@cbornet, #951). However with yaml 1.2 the situation will change somewhere in the future, but then a new state would not harm as well.

The behavior can be reproduced by removing NAKADI_OAUTH2_MODE=OFF from docker-compose.yml and adding OFF to application.yml.

@adyach
Copy link
Member

adyach commented Apr 23, 2019

@fbrns I do not understand why it is a problem to put it in quotes. From the other hand adding another identical auth mode looks like duplication and creates complication in understanding what purpose it serves. This state was created specifically for testing/debugging purposes thus it is used in conjunction with docker compose.

@fbrns
Copy link
Member Author

fbrns commented Nov 29, 2019

The functionality is not needed, therefore I close the pull-request.

@fbrns fbrns closed this Nov 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants