fix: Fix example configs and README.md file #2215
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
onidoru
requested review from
rchincha,
andaaron and
eusebiu-constantin-petu-dbk
as code owners
andaaron
reviewed
Jan 31, 2024
onidoru
force-pushed
the
change_example_ldap_configs
branch
from
d6cd646 to
7a4e289
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2215 +/- ##
==========================================
- Coverage 92.80% 92.78% -0.02%
==========================================
Files 166 166
Lines 21798 21811 +13
==========================================
+ Hits 20230 20238 +8
- Misses 980 984 +4
- Partials 588 589 +1 ☔ View full report in Codecov by Sentry. |
onidoru
force-pushed
the
change_example_ldap_configs
branch
2 times, most recently
from
95eca03 to
b3035fa
Compare
andaaron
reviewed
Feb 6, 2024
onidoru
force-pushed
the
change_example_ldap_configs
branch
from
3f38664 to
3528253
Compare
andaaron
reviewed
Feb 16, 2024
|
|
||
| return config.LDAPCredentials{}, err | ||
| } | ||
|
|
||
| if len(metaData.Keys) == 0 { | ||
| log.Error().Err(zerr.ErrBadConfig). |
There was a problem hiding this comment.
Can you cover this case? Should be easy enough.
| return config.LDAPCredentials{}, zerr.ErrBadConfig | ||
| } | ||
|
|
||
| if len(metaData.Unused) > 0 { |
There was a problem hiding this comment.
Can you cover this with a test case?
Signed-off-by: onidoru <[email protected]> Signed-off-by: Nikita Kotikov <[email protected]>
Signed-off-by: onidoru <[email protected]> Signed-off-by: Nikita Kotikov <[email protected]>
Signed-off-by: onidoru <[email protected]> Signed-off-by: Nikita Kotikov <[email protected]>
onidoru
force-pushed
the
change_example_ldap_configs
branch
from
a7ce686 to
8a61bbc
Compare
andaaron
reviewed
Feb 16, 2024
| content := []byte(`{ | ||
| "bindDN":"cn=ldap-searcher,ou=Users,dc=example,dc=org", | ||
| "bindPassword":"ldap-searcher-password", | ||
| "extraKey": "extraValue" |
There was a problem hiding this comment.
For some reason the test does not cover the lines we expected. Also the comment at line 1326 should probably be removed,
| defer os.Remove(tmpCredsFile.Name()) | ||
|
|
||
| // `bindDN` key is missing | ||
| content := []byte(``) |
There was a problem hiding this comment.
I think this case covers the error on viperInstance.UnmarshalExact( which is good, but we still need a separate test case with a valid empty json content := []byte({}).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
documentation
Which issue does this PR fix:
Example LDAP configs do not represent the actual state of things and are outdated.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.