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

[Fleet] Add elastic-api-version support #2677

Merged
merged 15 commits into from Jun 12, 2023
Merged

[Fleet] Add elastic-api-version support #2677

merged 15 commits into from Jun 12, 2023

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Jun 7, 2023

Description

Introduce a new middleware that support elastic-apiversion header to allow versioning of the Fleet server APIs.

If the header is not provided we fallback to a default version.

For PR readability I will add some integration tests that duplicate the client and the api tests suite in a following PR.

Todo

  • Unit tests the version middleware

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 7, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-06-12T13:31:43.621+0000

  • Duration: 38 min 16 sec

Test stats 🧪

Test Results
Failed 0
Passed 716
Skipped 1
Total 717

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

DefaultVersion = "2023-06-01"
)

var SupportedVersions = []string{DefaultVersion}
Copy link
Member Author

Choose a reason for hiding this comment

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

I am wondering if there is a better place to list the versions we support and the default version.

@nchaulet nchaulet added the Team:Fleet Label for the Fleet team label Jun 8, 2023
@nchaulet nchaulet self-assigned this Jun 8, 2023
@nchaulet nchaulet marked this pull request as ready for review June 8, 2023 14:22
@nchaulet nchaulet requested a review from a team as a code owner June 8, 2023 14:22
Copy link
Contributor

@michel-laterman michel-laterman left a comment

Choose a reason for hiding this comment

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

think it looks good, mostly have nitpicks.

We may want to consider altering the openapi spec to show responses will contain the version header (we don't do this for request ids, but we should)

The only real concern I have is now we're changing the behaviour of the status endpoint as well

internal/pkg/api/apiVersion.go Outdated Show resolved Hide resolved
internal/pkg/api/apiVersion.go Outdated Show resolved Hide resolved
internal/pkg/api/apiVersion.go Outdated Show resolved Hide resolved
model/openapi.yml Show resolved Hide resolved
Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com>
@nchaulet
Copy link
Member Author

nchaulet commented Jun 8, 2023

The only real concern I have is now we're changing the behaviour of the status endpoint as well

in my opinion is probably okay as if you do not provide the Elastic-Api-Version it will still work

internal/pkg/api/apiVersion.go Outdated Show resolved Hide resolved
internal/pkg/api/apiVersion.go Show resolved Hide resolved
internal/pkg/api/apiVersion.go Outdated Show resolved Hide resolved
@mergify
Copy link
Contributor

mergify bot commented Jun 9, 2023

This pull request is now in conflicts. Could you fix it @nchaulet? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b feature-api-version upstream/feature-api-version
git merge upstream/main
git push upstream feature-api-version

@nchaulet nchaulet merged commit ce791de into main Jun 12, 2023
18 checks passed
@nchaulet nchaulet deleted the feature-api-version branch June 12, 2023 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Fleet Label for the Fleet team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants