Skip to content

Conversation

@P1llus
Copy link
Member

@P1llus P1llus commented Mar 10, 2022

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

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.
@P1llus P1llus added bug Something isn't working, use only for issues Team:Security-External Integrations Integration:httpjson Custom API labels Mar 10, 2022
@P1llus P1llus requested a review from a team as a code owner March 10, 2022 10:07
@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@@ -1,3 +1,8 @@
- version: "1.1.0"
Copy link
Contributor

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?

Copy link
Member Author

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 ?

Copy link
Member

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?

@P1llus
Copy link
Member Author

P1llus commented Mar 10, 2022

Tested some of the non-working functionality like split manually against a MS API and working correctly now.

@P1llus P1llus requested a review from andrewkroh March 10, 2022 10:33
@elasticmachine
Copy link

elasticmachine commented Mar 10, 2022

💚 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](https://ci-stats.elastic.co/app/apm/services/beats-ci/transactions/view?rangeFrom=2022-03-10T14:47:03.067Z&rangeTo=2022-03-10T15:07:03.067Z&transactionName=BUILD Ingest-manager/integrations/PR-{number}&transactionType=job&latencyAggregationType=avg&traceId=014c671356b35a9838fbec9dae94438d&transactionId=e79f35f04b55d8a1)

Expand to view the summary

Build stats

  • Start Time: 2022-03-10T14:57:03.067+0000

  • Duration: 15 min 54 sec

Test stats 🧪

Test Results
Failed 0
Passed 6
Skipped 0
Total 6

🤖 GitHub comments

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

  • /test : Re-trigger the build.
Copy link
Member

@andrewkroh andrewkroh left a 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"
Copy link
Member

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?

@P1llus
Copy link
Member Author

P1llus commented Mar 10, 2022

Please attach a screenshot of the config options in the UI.

Update tested:
image

Basic:
image

Transforms:
image

Response:
image

Request Custom:
image

Oauth custom:
image

General:
image

auth.oauth2.azure.resource: {{oauth_azure_resource}}
{{/if}}
{{#if oauth_endpoint_params}}
auth.oauth2.azure.resource:
Copy link

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?

Copy link
Member Author

@P1llus P1llus Mar 24, 2022

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

Copy link

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!

Copy link
Member Author

Choose a reason for hiding this comment

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

This is being resolved in 1.1.1 @trengrj , which should be available a few hours after this is merged: #2883

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working, use only for issues Integration:httpjson Custom API

5 participants