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

docker: fixes spring config in zipkin-eureka test image #3699

Merged
merged 6 commits into from
Jan 23, 2024

Conversation

codefromthecrypt
Copy link
Member

Formerly, we didn't test the authenticated side, so some review drift passed even though authenticating would crash on startup:

Parameter 0 of method filterChain in zipkin.test.EurekaSecurity required a bean of type 'org.springframework.security.config.annotation.web.builders.HttpSecurity' that could not be found.

Formerly, we didn't test the authenticated side, so some review drift
passed even though authenticating would crash on startup:

```
Parameter 0 of method filterChain in zipkin.test.EurekaSecurity required a bean of type 'org.springframework.security.config.annotation.web.builders.HttpSecurity' that could not be found.
```

Signed-off-by: Adrian Cole <[email protected]>
# Test both authenticated and unauthenticated, as if there is a Spring problem,
# the latter will crash. We only need to use HEALTHCHECK for this.
services:
eureka:
Copy link
Member Author

@codefromthecrypt codefromthecrypt Jan 23, 2024

Choose a reason for hiding this comment

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

without a compose, it effectively only runs this part. I daisy-chained the two paths since we are using spring security for username/password stuff. Maybe someone can convert this to armeria and use normal simple basic auth filter one day, but I went with the spring-security route initially.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry missed that

Copy link
Member Author

Choose a reason for hiding this comment

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

no worries I should have checked. Anyway the prior one failed on health check for frustrating reasons.

Adrian Cole added 2 commits January 23, 2024 08:10
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
@@ -33,7 +33,7 @@
@Configuration
@ConditionalOnProperty("eureka.username")
@EnableConfigurationProperties(EurekaProperties.class)
@ImportAutoConfiguration(SecurityAutoConfiguration.class)
@Import(SecurityAutoConfiguration.class)
Copy link
Contributor

Choose a reason for hiding this comment

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

So this one didn't work out? (just curious since we're importing auto configuration)

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah it threw the error I mentioned in the description. Then, I fought for an hour over CSRF. Finally, I axed this thing for a basic filter (pun intended)

Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt
Copy link
Member Author

I removed spring-security as it was far more effort than value to just verify basic auth.

Adrian Cole added 2 commits January 23, 2024 09:44
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt
Copy link
Member Author

cool finally passing ;) now I will forget all the config lambda-foo and try to remember what I was doing

@@ -50,15 +50,6 @@
<artifactId>spring-cloud-starter-netflix-eureka-server</artifactId>
</dependency>

<dependency>
<groupId>org.springframework.security</groupId>
Copy link
Member Author

Choose a reason for hiding this comment

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

note this didn't change the image size as even if the eureka layer is 55mb the current classpath requires modules not in our JRE, so we use the JDK image as noted in the Dockerfile. The image sum with the JDK layer ends up as 430MB. Maybe a trimmer eureka will happen one day, but it isn't terribly important.

@codefromthecrypt codefromthecrypt merged commit 10e0458 into master Jan 23, 2024
13 checks passed
@codefromthecrypt codefromthecrypt deleted the fix-eureka branch January 23, 2024 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants