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

headers filters - custom service configuration should override default headers configuration (#120) #142

Merged
merged 4 commits into from
Sep 12, 2024

Conversation

pmauduit
Copy link
Member

@pmauduit pmauduit commented Sep 10, 2024

Fixes #120


Fix custom service header mappings not being applied

Custom service header mappings not applied due to Spring Boot not correctly parsing Service.headers: Optional<HeaderMappings>.

Apparently using Optional for @ConfigurationProperties is ok for scalars but not for nested objects.

So a service header definitions like

          datafeeder:
            target: ${georchestra.gateway.services.datafeeder.target}
            headers:
              json-user: true
              json-organization: true

Wouldn't get the json-user and json-organization request headers applied to downstream services.

pmauduit and others added 2 commits September 10, 2024 17:30
Custom service header mappings not applied due to Spring Boot
not correctly parsing `Service.headers: Optional<HeaderMappings>`.

Apparently using `Optional` for `@ConfigurationProperties` is ok for
scalars but not for nested objects.

So a service header definitions like

```
      datafeeder:
        target: ${georchestra.gateway.services.datafeeder.target}
        headers:
          json-user: true
          json-organization: true
```

Wouldn't get the json-user and json-organization request headers applied
to downstream services.
@groldan groldan marked this pull request as ready for review September 10, 2024 21:50
@groldan
Copy link
Member

groldan commented Sep 10, 2024

Note the build failure:

Error:    SendMessageRabbitmqIT.testReceivingMessageFromConsole:119 » ConditionTimeout Condition org.georchestra.gateway.rabbitmq.SendMessageRabbitmqIT$$Lambda/0x00007f2700bed888 was not fulfilled within 30 seconds.

is unrelated and seems to be there since a while ago

@pmauduit
Copy link
Member Author

SendMessageRabbitmqIT

yes we noticed it, we might just deactivate for now and revisit later (from what I investigated it fails because it relies on the console container output, so if something changes in the way the console is logging, then the test fails).

Copy link
Member Author

@pmauduit pmauduit left a comment

Choose a reason for hiding this comment

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

Thanks @groldan for completing the PR ! Looks good to me overall, I had just few comments reviewing it

Check the account creation onto the LDAP instead of parsing the console
output.
@pmauduit
Copy link
Member Author

is unrelated and seems to be there since a while ago

added a commit to deactivate it. Reading the output, it seems that the issue is console-side.

Note: with a console docker image compiled with
georchestra/georchestra#4332 the test passes.
@pmauduit
Copy link
Member Author

added a commit to deactivate it. Reading the output, it seems that the issue is console-side.

With georchestra/georchestra#4332 we could reactivate the test.

@pmauduit pmauduit merged commit cecbf43 into main Sep 12, 2024
3 checks passed
@pmauduit pmauduit deleted the custom-headers-config-issue-120 branch September 12, 2024 17:02
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.

custom headers configuration on targets is ignored ?
2 participants