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.

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

Control service: remove deprecated dependency #1589

Merged

Conversation

murphp15
Copy link
Collaborator

@murphp15 murphp15 commented Feb 2, 2023

Why

At the moment we are blocked on spring upgrades because we are using a number of legacy classes. #1530
This remove one of those dependencies.
Even after this is merged I don't expect that PR to be fixed as there will be a number of other issues but this is a good first step

What

I followed the guidelines here on how to remove it https://spring.io/blog/2022/02/21/spring-security-without-the-websecurityconfigureradapter

Closes: #1590

Signed-off-by: murphp15 murphp15@tcd.ie

murphp15 and others added 2 commits February 2, 2023 16:22
Signed-off-by: murphp15 <murphp15@tcd.ie>
murphp15 and others added 4 commits February 2, 2023 16:23
Signed-off-by: murphp15 <murphp15@tcd.ie>
Signed-off-by: murphp15 <murphp15@tcd.ie>
@murphp15 murphp15 changed the title draft control service. remove deprecated dependency Control service: remove deprecated dependency Feb 3, 2023
@DeltaMichael
Copy link
Contributor

This seems to follow the migration guide closely, so no problem with the code itself. I'm just curious if and how the security config is tested.

Copy link
Contributor

@ivakoleva ivakoleva left a comment

Choose a reason for hiding this comment

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

mind that merging may then auto-close the issue;

@murphp15
Copy link
Collaborator Author

murphp15 commented Feb 6, 2023

This seems to follow the migration guide closely, so no problem with the code itself. I'm just curious if and how the security config is tested.

The test KerberosAuthenticationIT tests the kerb authentication.
But also we deploy to a k8s cluster as part of the CICD pipelines which would also test the auth.
Let me know if you have further questions!?

@mivanov1988
Copy link
Contributor

LGTM

murphp15 and others added 2 commits February 6, 2023 16:45
Signed-off-by: murphp15 <murphp15@tcd.ie>
@murphp15 murphp15 enabled auto-merge (squash) February 6, 2023 16:48
@murphp15 murphp15 merged commit 34c6f82 into main Feb 6, 2023
@murphp15 murphp15 deleted the person/murphp15/remove_dependency_on_deprecated_class branch February 6, 2023 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Control service: Remove dependency on deprecated class WebSecurityConfigurerAdapter
5 participants