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

Various updates #18

Closed
wants to merge 15 commits into from
Closed

Various updates #18

wants to merge 15 commits into from

Conversation

dzarras
Copy link
Contributor

@dzarras dzarras commented Nov 17, 2023

  1. Adds dependency to Spring Actuator and exposes health endpoint
  2. Cleans up build.gradle.kts
  3. Updates folder structure for docker compose
  4. Simplifies configuration of insecure WebClients
  5. Adds PID Issuer application in docker-compose.yaml
  6. Adds documentation for docker compose
  7. Adds documentation for the environment variables that can be used to configured PID issuer

Closes: #4

@dzarras dzarras added documentation Improvements or additions to documentation feature New feature or request labels Nov 17, 2023
@dzarras dzarras requested a review from babisRoutis November 17, 2023 15:33
@dzarras dzarras requested a review from a team as a code owner November 17, 2023 15:33
dzarras added 15 commits November 17, 2023 17:41
@dzarras @babisRoutis
remove unused property.
f873527
@dzarras @babisRoutis
Add the ability to expose the Authorization Server using a different …
d480f12 
…URL in Credential Issuer Metadata.
@dzarras @babisRoutis
Add and enable Spring Actuator's Health endpoint.
a0d0a63
@dzarras @babisRoutis
Cleanup build.gradle.kts
1069a2e
@dzarras @babisRoutis
Add pid-issuer to docker-compose.
e98ee33
@dzarras @babisRoutis
Enable TLS termination in Keycloak to allow introspection and userinf…
af923e5 
…o endpoint to work without issues.
@dzarras @babisRoutis
Revert "Add the ability to expose the Authorization Server using a di…
94e132d 
…fferent URL in Credential Issuer Metadata."

This reverts commit c5dacef.
@dzarras @babisRoutis
Update folder structure for docker-compose local dev environment.
0068151
@dzarras @babisRoutis
Update documentation for Keycloak in docker compose.
d38c566
@dzarras @babisRoutis
Provide a WebClient bean based on the active Spring Profile.
9a416f4
@dzarras @babisRoutis
Always set 'FC' as the country in EncodePidInCborWithMicroService.
7a31fbe
@dzarras @babisRoutis
Prefer using environment variables to configured pid-issuer in docker…
7bd5532 
…-compose.yaml
@dzarras @babisRoutis
Further document docker-compose.
61bea5b
@dzarras @babisRoutis
Consider WebFlux base path in default public URL.
894c107
@dzarras @babisRoutis
Document the environment variables that can be used to configure the …
1a92133 
…PID issuer application.
babisRoutis
babisRoutis requested changes Nov 17, 2023
View reviewed changes
Copy link
Contributor

@babisRoutis babisRoutis left a comment

Choose a reason for hiding this comment

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

@dzarras

Added some minor comments.
Let's discuss them

src/main/resources/application-prod.properties
#
# Issuer options
#
issuer.clientId=pid-issuer
issuer.publicUrl=http://localhost:${server.port}
issuer.publicUrl=http://localhost:${server.port}/${spring.webflux.base-path}
Copy link
Contributor

Choose a reason for hiding this comment

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

This will evaluate to // I think

src/main/kotlin/eu/europa/ec/eudi/pidissuer/PidIssuerApplication.kt
@@ -246,9 +241,12 @@ fun beans(clock: Clock) = beans {
authorize(MetaDataApi.WELL_KNOWN_OPENID_CREDENTIAL_ISSUER, permitAll)
authorize(MetaDataApi.WELL_KNOWN_JWKS, permitAll)
authorize(IssuerApi.CREDENTIALS_OFFER, permitAll)
authorize("/actuator/health", permitAll)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not All

src/main/kotlin/eu/europa/ec/eudi/pidissuer/PidIssuerApplication.kt
},
)
}
bean {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this inside the security configuration if profile can be nested inside a beans { }

src/main/kotlin/eu/europa/ec/eudi/pidissuer/adapter/out/pid/EncodePidInCborWithMicroService.kt
class EncodePidInCborWithMicroService(private val creatorUrl: HttpsUrl) : EncodePidInCbor {
class EncodePidInCborWithMicroService(
private val creatorUrl: HttpsUrl,
private val webClient: WebClient,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why pass a client?

Pass the "Mode of operation" a let the adapter provision a WebClient using the factory methods

@babisRoutis
Copy link
Contributor

In general to mix to many changes in PR
For instance actuator is irrelevant to the main subject of the PR

@dzarras dzarras closed this Nov 20, 2023
@dzarras dzarras deleted the feature/add-pid-issuer-to-compose branch November 20, 2023 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@babisRoutis babisRoutis babisRoutis requested changes

Assignees

@babisRoutis babisRoutis

Labels
documentation Improvements or additions to documentation feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Document docker-compose
2 participants
@dzarras @babisRoutis