Skip to content

Align Cloud Controller response handling with Spring, improve tests#1521

Merged
bart-vmware merged 15 commits into
mainfrom
cf_permissions
Jun 12, 2025
Merged

Align Cloud Controller response handling with Spring, improve tests#1521
bart-vmware merged 15 commits into
mainfrom
cf_permissions

Conversation

@TimHess
Copy link
Copy Markdown
Member

@TimHess TimHess commented May 29, 2025 •
edited
Loading

Description

To answer the original question: when using Spring Boot as the spec for these interactions, HTTP 503 can be the "correct" response.

This PR:

  • Adds testing of various potential cloud controller responses
  • Allows usage of 401 response code (all 4xx errors besides 403) | relevant Spring code
  • Allows usage of 500 response code, with error message in response | relevant Spring code

Resolves #1446

Quality checklist

  • Your code complies with our Coding Style.
  • You've updated unit and/or integration tests for your change, where applicable.
  • You've updated documentation for your change, where applicable.
    If your change affects other repositories, such as Documentation, Samples and/or MainSite, add linked PRs here.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.
  • You've added required license files and/or file headers (explaining where the code came from with proper attribution), where code is copied from StackOverflow, a blog, or OSS.

bart-vmware
bart-vmware reviewed May 30, 2025
View reviewed changes
Comment thread src/Management/src/Endpoint/Actuators/CloudFoundry/PermissionsProvider.cs Outdated
bart-vmware
bart-vmware requested changes Jun 2, 2025
View reviewed changes
Comment thread src/Management/src/Endpoint/Actuators/CloudFoundry/PermissionsProvider.cs Outdated
Comment thread src/Management/src/Endpoint/Actuators/CloudFoundry/PermissionsProvider.cs Outdated
Comment thread src/Management/src/Endpoint/Actuators/CloudFoundry/PermissionsProvider.cs
Comment thread src/Management/src/Endpoint/Actuators/CloudFoundry/PermissionsProvider.cs Outdated
Comment thread src/Management/src/Endpoint/Actuators/CloudFoundry/PermissionsProvider.cs Outdated
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/CloudFoundrySecurityMiddlewareTest.cs Outdated
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/CloudFoundrySecurityMiddlewareTest.cs Outdated
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/PermissionsProviderTest.cs Outdated
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/PermissionsProviderTest.cs Outdated
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/StartupWithSecurity.cs Outdated
@TimHess TimHess marked this pull request as ready for review June 2, 2025 21:34
bart-vmware
bart-vmware reviewed Jun 3, 2025
View reviewed changes
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/CloudFoundrySecurityMiddlewareTest.cs Outdated
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/PermissionsProviderTest.cs Outdated
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/PermissionsProviderTest.cs Outdated
@TimHess TimHess requested a review from bart-vmware June 5, 2025 19:41
@TimHess TimHess self-assigned this Jun 5, 2025
@TimHess TimHess added Component/Management Issues related to Steeltoe Management (actuators) ReleaseLine/4.x Identified as a feature/fix for the 4.x release line labels Jun 5, 2025
@TimHess TimHess added this to the 4.0.0-rc1 milestone Jun 5, 2025
bart-vmware
bart-vmware reviewed Jun 6, 2025
View reviewed changes
Comment thread src/Common/src/Common/Extensions/ExceptionExtensions.cs Outdated
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/CloudFoundrySecurityMiddlewareTest.cs Outdated
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/PermissionsProviderTest.cs Outdated
@TimHess TimHess requested a review from bart-vmware June 6, 2025 14:49
bart-vmware
bart-vmware requested changes Jun 10, 2025
View reviewed changes
Comment thread src/Management/test/Endpoint.Test/Actuators/CloudFoundry/CloudControllerPermissionsMock.cs Outdated
bart-vmware
bart-vmware reviewed Jun 10, 2025
View reviewed changes
Comment thread src/Management/src/Endpoint/Actuators/CloudFoundry/CloudFoundrySecurityMiddleware.cs Outdated
@TimHess TimHess requested a review from bart-vmware June 11, 2025 15:55
bart-vmware
bart-vmware approved these changes Jun 12, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bart-vmware bart-vmware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

TimHess and others added 12 commits June 12, 2025 10:32
@TimHess @bart-vmware
Align Cloud Controller response handling with Spring, improve tests
9d1b01a
@TimHess @bart-vmware
code cleanup
78bdc9b
@TimHess @bart-vmware
remove catching timeouts, add log assertions
88c6c49
@TimHess @bart-vmware
catch cancel/timeout again, with trace-log and rethrow
e662a7b
@TimHess @bart-vmware
PR feedback
17ab4d2 
- Use Microsoft.Net.Http.Headers.HeaderNames.Authorization
- Move permissions messages to static class
- Rename to ParsePermissionsResponseAsync
- test cleanup
@TimHess @bart-vmware
Update src/Management/test/Endpoint.Test/Actuators/CloudFoundry/Permi…
6606643 
…ssionsProviderTest.cs

Co-authored-by: Bart Koelman <104792814+bart-vmware@users.noreply.github.com>
@TimHess @bart-vmware
PR feedback
8d28290
@TimHess @bart-vmware
back to not catching cancellation, use real web host and other test c…
2145031 
…leanup
@TimHess @bart-vmware
Catch timeouts on permissions check
505a278 
- move integration scenarios to dedicated class
- allow assert on presence of multiple log entries per scenario
- use a mock on RedactsHttpHeaders now that exceptions aren't caught
@TimHess @bart-vmware
naming things, use typeof for logs, comments
df9b462
@TimHess @bart-vmware
remove inner log, formatting
93b330b
@TimHess @bart-vmware
catch HttpRequestException
ff1d780
@bart-vmware bart-vmware merged commit 86ae318 into main Jun 12, 2025
25 checks passed
@bart-vmware bart-vmware deleted the cf_permissions branch June 12, 2025 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bart-vmware bart-vmware bart-vmware approved these changes

Assignees

@TimHess TimHess

Labels

Component/Management Issues related to Steeltoe Management (actuators) ReleaseLine/4.x Identified as a feature/fix for the 4.x release line

Projects

None yet

Milestone

4.0.0-rc1

Development

Successfully merging this pull request may close these issues.

Is the 503 status code correct when the request to check permissions in CF fails

2 participants

@TimHess @bart-vmware