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

fix(trait): use a configmap index #5948

Merged
merged 1 commit into from
Nov 20, 2024
Merged

fix(trait): use a configmap index #5948

merged 1 commit into from
Nov 20, 2024

Conversation

squakez
Copy link
Contributor

@squakez squakez commented Nov 16, 2024

Closes #5924

Release Note

fix(trait): use a configmap index

@squakez
Copy link
Contributor Author

squakez commented Nov 16, 2024

fyi @hernanDatgDev I've tested this against your reproducer and it seems the root issue is fixed. Please, have a look.

@hernanDatgDev
Copy link
Contributor

@squakez I pulled your fork and built the project locally. For some reason I'm getting a rest dsl plugin failure when my integration is building.
I've been struggling with the root cause. Is there some difference between v2.4 and 2.5 RE how the project is built or the api spec that's required for this to work? I feel like I must be missing something obvious.

{"level":"error","ts":"2024-11-19T03:31:47Z","logger":"camel-k.maven.build","msg":"Failed to execute goal org.apache.camel.k:camel-k-maven-plugin:3.15.0:generate-rest-xml (default) on project camel-k-rest-d │
│ sl-generator: Exception while generating rest xml: Cannot invoke \"io.swagger.v3.oas.models.media.Content.entrySet()\" because the return value of \"io.swagger.v3.oas.models.responses.ApiResponse.getContent()\" is null -> [Help 1]","stac │
│ ktrace":"github.com/apache/camel-k/v2/pkg/util/log.Logger.Error\n\tgithub.com/apache/camel-k/v2/pkg/util/log/log.go:80\ngithub.com/apache/camel-k/v2/pkg/util/maven.normalizeLog\n\tgithub.com/apache/camel-k/v2/pkg/util/maven/maven_log.go: │
│ 87\ngithub.com/apache/camel-k/v2/pkg/util/maven.LogHandler\n\tgithub.com/apache/camel-k/v2/pkg/util/maven/maven_log.go:53\ngithub.com/apache/camel-k/v2/pkg/util.scan\n\tgithub.com/apache/camel-k/v2/pkg/util/command.go:76\ngithub.com/apac │
│ he/camel-k/v2/pkg/util.RunAndLog.func1\n\tgithub.com/apache/camel-k/v2/pkg/util/command.go:55\ngolang.org/x/sync/errgroup.(*Group).Go.func1\n\tgolang.org/x/[email protected]/errgroup/errgroup.go:78"}

For reference here's the api spec I'm including in my referenced configmap:

    openapi: 3.0.3
    info:
      title: Basic API
      version: "1.0"
    paths:
      /test:
        post:
          operationId: test
          responses:
            200:
              description: Default response
      /user/{userId}:
        get:
          operationId: getUser
          parameters:
            - name: userId
              in: path
              required: true
              schema:
                type: string
          responses:
            200:
              description: Default response

@squakez
Copy link
Contributor Author

squakez commented Nov 19, 2024

Hi @hernanDatgDev, please, let's stick to the original reproducer reported in #5924 (comment) to avoid mixing up things. I've removed the response content and I manage to reproduce your same problem with runtime 3.15.0 (Camel core 4.8.0). However, this is a different problem from the one we're fixing in this PR.

I've run the same reproducer against runtime 3.8.1 and the issue is not appearing, so, there is some regression or new behavior introduced in the new Camel core 4.8.0. From Camel K perspective we can't really do anything until that is fixed in the core unfortunately. I suggest you please open an issue and report this in the core. It also may be a core fix, given that we may expect a response to always provide a content, but I am not into openapi spec details to confirm that.

As for this PR, please, feel free to have a look and verify if the original issue is solved, so I can merge accordingly.

@squakez
Copy link
Contributor Author

squakez commented Nov 19, 2024

It could be caused by the removal of a swagger validator [1] or even the upgrade of swagger dependencies which may be adding now some validation to avoid such value to be empty.

https://camel.apache.org/manual/camel-4x-upgrade-guide-4_6.html#_camel_rest_openapi

@hernanDatgDev
Copy link
Contributor

@squakez Yes you're right there is some additional validation on the openapi spec and it expects certain parts to be present. I made the necessary changes and got the expected behavior. I also got the expected behavior with the original files I provided in my ticket. The changes made seem good to me so far 👍

@squakez squakez merged commit 08ecf06 into apache:main Nov 20, 2024
10 checks passed
@squakez squakez deleted the feat/5924 branch November 20, 2024 16:58
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.

Mount trait regression with release 2.5.0
3 participants