- Notifications
You must be signed in to change notification settings - Fork 513
[HTTPJSON] fixing config fields in manifest.yml not working as intended #2815
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
Conversation
| Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
| @@ -1,3 +1,8 @@ | |||
| - version: "1.1.0" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Since some options were removed, this shouldn't be a major bump? Or if we consider it a fix, a patch bump?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All options now has their own field, so its not removed per se, but the user will get a UI warning asking them to fill in the fields again, I can still bump major if you want @marc-gr ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested the upgrade process for this package version and verified that you can resolve the conflicts without any show stopping Fleet UI errors?
| Tested some of the non-working functionality like split manually against a MS API and working correctly now. |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
andrewkroh left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please attach a screenshot of the config options in the UI.
| @@ -1,3 +1,8 @@ | |||
| - version: "1.1.0" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested the upgrade process for this package version and verified that you can resolve the conflicts without any show stopping Fleet UI errors?
| auth.oauth2.azure.resource: {{oauth_azure_resource}} | ||
| {{/if}} | ||
| {{#if oauth_endpoint_params}} | ||
| auth.oauth2.azure.resource: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@P1llus shouldn't this be auth.oauth2.endpoint_params instead of auth.oauth2.azure.resource? Do you want me to raise an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing it out, I don't need an issue for this, il push a fix later today, you should be able to see the change later today :+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks that would be much appreciated!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.







What does this PR do?
This removes oauth_custom and request_custom, and rather have separate UI fields for each option, to resolve issues with how config templates are converted in kibana.
Checklist
changelog.ymlfile.