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.

Sign up for GitHub

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

Jump to bottom

WebhookAuthenticator .Status and validation improvements #1894

Merged
merged 17 commits into from
Mar 27, 2024
Merged

WebhookAuthenticator .Status and validation improvements #1894

merged 17 commits into from
Mar 27, 2024

Conversation

benjaminapetersen
Copy link
Member

@benjaminapetersen benjaminapetersen commented Mar 13, 2024 •
edited
Loading

Release note:

Adds meaningful .Status to WebhookAuthenticator resource.  Improves validations.

# .Status was previously an empty field.
# full success .Status now looks like this:
status:
  phase: Ready
  conditions:
  - lastTransitionTime: "2024-02-23T19:31:36Z"
    message: authenticator initialized
    observedGeneration: 1
    reason: Success
    status: "True"
    type: AuthenticatorValid 
  - lastTransitionTime: "2024-02-23T19:31:36Z"
    message: the WebhookAuthenticator is ready
    observedGeneration: 1
    reason: Success
    status: "True"
    type: Ready
  - lastTransitionTime: "2024-02-23T19:31:33Z"
    message: successfully parsed specified CA bundle
    observedGeneration: 1
    reason: Success
    status: "True"
    type: TLSConfigurationValid 
  - lastTransitionTime: "2024-02-23T19:31:33Z"
    message: tls verified
    observedGeneration: 1
    reason: Success
    status: "True"
    type: WebhookConnectionValid     
  - lastTransitionTime: "2024-02-23T19:31:33Z"
    message: endpoint is a valid URL
    observedGeneration: 1
    reason: Success
    status: "True"
    type: EndpointURLValid

@codecov Codecov
Copy link

codecov bot commented Mar 13, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 80.39773% with 69 lines in your changes are missing coverage. Please review.

Project coverage is 38.57%. Comparing base (4465810) to head (c6b0820).

Files Patch % Lines
test/testlib/client.go 0.00% 37 Missing ⚠️
...enticator/webhookcachefiller/webhookcachefiller.go 94.65% 12 Missing and 1 partial ⚠️
internal/controller/utilserr.go 0.00% 9 Missing ⚠️
internal/testutil/stringutil.go 0.00% 5 Missing ⚠️
internal/controllermanager/prepare_controllers.go 0.00% 4 Missing ⚠️
...onfig/oidcupstreamwatcher/oidc_upstream_watcher.go 66.66% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1894      +/-   ##
==========================================
+ Coverage   38.25%   38.57%   +0.32%     
==========================================
  Files         347      350       +3     
  Lines       44207    44514     +307     
==========================================
+ Hits        16910    17171     +261     
- Misses      26785    26828      +43     
- Partials      512      515       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@benjaminapetersen benjaminapetersen changed the title Ben/status/webhook authenticator WebhookAuthenticator .Status and validation improvements Mar 13, 2024
@benjaminapetersen
Copy link
Member Author

Cross reference to the JWT Authenticator status and validations as they are related:

#1851

cfryanr
cfryanr reviewed Mar 13, 2024
View reviewed changes
test/testlib/client.go Outdated Show resolved Hide resolved
benjaminapetersen and others added 9 commits March 19, 2024 16:48
@benjaminapetersen
Add WebhookAuthenticator integration tests, expand unit tests
590e2d1 
- Add WebhookAuthenticator unit tests, update generated code
- Add validateTLSNegotiation(), update tests
- Update validateTLSNegotiation, add unit tests, factor out helpers
- Update generated code
@benjaminapetersen
Update webhook status integration tests
337459f 
- total api fields test 260->261
@benjaminapetersen
Adjust testlib/client.go for lint quirk
5c1fa6d
@benjaminapetersen
Refactor logLines to SplitByNewline, deduplicate
0467e5c
@benjaminapetersen
Improve JWTAuthenticator validation of Issuer,Discovery
a45a537
@benjaminapetersen
Always pass spec to CreateTestWebhookAuthenticator
097e6d5
@benjaminapetersen
add WebhookCacheFiller updateStatus tests
b6512bc
@benjaminapetersen
refactor WebhookAuthenticator newWebhookAuthenticator func
5c0d67d
@joshuatcasey @benjaminapetersen
Add IPv6 test to WebhookAuthenticator ctrl tests
90e7343 
Co-authored-by: Benjamin A. Petersen <ben@benjaminapetersen.me>
benjaminapetersen and others added 3 commits March 26, 2024 15:32
@benjaminapetersen @cfryanr
Update ParseFromURL usage comment.
eed0c9d 
- Carefully note the rfc27732 design for IPv6 in URLs, while also clarifying the handling of IPv6 in Golang.

Co-authored-by: Ryan Richard <richardry@vmware.com>
@benjaminapetersen
Update WebhookAuthenticator Status WebhookConnectionValid
f86c46e 
- ConnectionProbeValid -> WebhookConnectionValid
  - This is to conform with the pattern of other controllers, ex:
    LDAPConnectionValid
@benjaminapetersen
Fix some utils, spacing, func naming, test inputs, etc.
c6b0820
@benjaminapetersen benjaminapetersen merged commit f0a43e0 into main Mar 27, 2024
40 checks passed
@benjaminapetersen benjaminapetersen deleted the ben/status/webhook-authenticator branch March 27, 2024 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cfryanr cfryanr cfryanr approved these changes

@joshuatcasey joshuatcasey joshuatcasey approved these changes

Assignees
No one assigned
Labels
cla-not-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@benjaminapetersen @joshuatcasey @cfryanr @vmwclabot