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

[Cloud Security] add is_internal config option for outputs #175546

Merged
merged 18 commits into from
Jan 31, 2024

Conversation

maxcold
Copy link
Contributor

@maxcold maxcold commented Jan 25, 2024

Summary

introducing a new is_internal config option for xpack.fleet.outputs. The usage is currently to protect the internal outputs in the UI:

  • filter out internal outputs in the Settings UI
  • disable internal outputs in output select for an agent policy

Screencast

screencast-github.com-2024.01.26-15_57_56.webm

Checklist

Delete any items that are not applicable to this PR.

  • Documentation was added for features that require explanation or tutorials
  • Unit or functional tests were updated or added to match the most common scenarios
  • If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list

@maxcold maxcold requested a review from a team January 25, 2024 14:12
@maxcold
Copy link
Contributor Author

maxcold commented Jan 26, 2024

/ci

@maxcold
Copy link
Contributor Author

maxcold commented Jan 26, 2024

/ci

@maxcold maxcold marked this pull request as ready for review January 26, 2024 14:59
@maxcold maxcold requested review from a team as code owners January 26, 2024 14:59
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jan 26, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@maxcold maxcold added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Cloud Security Cloud Security team related labels Jan 26, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

@@ -523,6 +523,7 @@
"hosts",
"is_default",
"is_default_monitoring",
"is_internal",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tbh not sure how to test/check this change, just following the pattern for other config keys

Copy link
Contributor

Choose a reason for hiding this comment

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

You manually modified this file right?

This check was added to make sure that any mapping addition is properly done, by adding a new model version. And I don't see that being done in this PR (looking at x-pack/plugins/fleet/server/saved_objects/index.ts)

Please follow the guidelines available in the model version documentation

(or follow what was already done for this SO type:

'2': {
changes: [
{
type: 'mappings_addition',
addedMappings: {
service_token: { type: 'keyword', index: false },
},
},
],
},
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the pointer, will check and follow the guidelines!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pgayvallet added a new model version to reflect the addition of the is_internal mapping. Btw running node scripts/check_mappings_update --fix made quite a lot of changes to the current_mappings.json which are not related to the is_internal, so I ended up still adding

      "is_internal": {
        "type": "boolean",
        "index": false
      },

manually (node scripts/check_mappings_update passes after I added the model version)

Copy link
Member

@kpollich kpollich left a comment

Choose a reason for hiding this comment

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

Code LGTM - one minor suggestion on the docs wording, otherwise 🚀

docs/settings/fleet-settings.asciidoc Outdated Show resolved Hide resolved
Copy link
Contributor

@kilfoyle kilfoyle left a comment

Choose a reason for hiding this comment

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

LGTM for the docs change. 👍

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

Mappings were added without corresponding model version

@@ -523,6 +523,7 @@
"hosts",
"is_default",
"is_default_monitoring",
"is_internal",
Copy link
Contributor

Choose a reason for hiding this comment

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

You manually modified this file right?

This check was added to make sure that any mapping addition is properly done, by adding a new model version. And I don't see that being done in this PR (looking at x-pack/plugins/fleet/server/saved_objects/index.ts)

Please follow the guidelines available in the model version documentation

(or follow what was already done for this SO type:

'2': {
changes: [
{
type: 'mappings_addition',
addedMappings: {
service_token: { type: 'keyword', index: false },
},
},
],
},
)

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

@maxcold
Copy link
Contributor Author

maxcold commented Jan 29, 2024

@nchaulet thanks for the pointer, added is_internal there and a unit test for that

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

code LGTM 🚀

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@eyalkraft eyalkraft left a comment

Choose a reason for hiding this comment

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

Nice!

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 1.2MB 1.2MB +76.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@maxcold maxcold merged commit 7b24ddd into main Jan 31, 2024
21 checks passed
@maxcold maxcold deleted the csp-hide-internal-output-and-fleet-server-host branch January 31, 2024 12:15
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 1, 2024
* main:
  use build hash in FTR tests
  [Security Solution] Fix moderate typo (elastic#175883)
  [Fleet] Fix conflicting dynamic template mappings for intermediate objects (elastic#175970)
  [Visualize] Prevent overwriting managed content (elastic#175274)
  [SLO] Add/edit form mark optional fields (elastic#175807)
  skip failing test suite (elastic#175984)
  [data views] Provide method of excluding data tiers when getting field list (elastic#167946)
  [Dataset quality] State management (elastic#174906)
  [Cloud Security] add is_internal config option for outputs (elastic#175546)
maxcold added a commit that referenced this pull request Feb 2, 2024
…tion (#175983)

## Summary

- Follow up after #175546
- Part of #165251

introducing a new `is_internal` config option for
`xpack.fleet.fleetServerHosts`. The usage is currently to protect the
internal fleet server hosts in the UI:

- filter them out in the Settings UI
- disable internal hosts in the agent policy form



### Checklist

Delete any items that are not applicable to this PR.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: David Kilfoyle <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Feb 7, 2024
…tion (elastic#175983)

## Summary

- Follow up after elastic#175546
- Part of elastic#165251

introducing a new `is_internal` config option for
`xpack.fleet.fleetServerHosts`. The usage is currently to protect the
internal fleet server hosts in the UI:

- filter them out in the Settings UI
- disable internal hosts in the agent policy form



### Checklist

Delete any items that are not applicable to this PR.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: David Kilfoyle <[email protected]>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…75546)

## Summary

- part of elastic#165251

introducing a new `is_internal` config option for `xpack.fleet.outputs`.
The usage is currently to protect the internal outputs in the UI:
- filter out internal outputs in the Settings UI 
- disable internal outputs in output select for an agent policy

### Screencast

[screencast-github.com-2024.01.26-15_57_56.webm](https://github.com/elastic/kibana/assets/478762/917b4a76-a48f-4bdc-b3d8-5598f86febf8)


### Checklist

Delete any items that are not applicable to this PR.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Kyle Pollich <[email protected]>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…tion (elastic#175983)

## Summary

- Follow up after elastic#175546
- Part of elastic#165251

introducing a new `is_internal` config option for
`xpack.fleet.fleetServerHosts`. The usage is currently to protect the
internal fleet server hosts in the UI:

- filter them out in the Settings UI
- disable internal hosts in the agent policy form



### Checklist

Delete any items that are not applicable to this PR.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: David Kilfoyle <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…75546)

## Summary

- part of elastic#165251

introducing a new `is_internal` config option for `xpack.fleet.outputs`.
The usage is currently to protect the internal outputs in the UI:
- filter out internal outputs in the Settings UI 
- disable internal outputs in output select for an agent policy

### Screencast

[screencast-github.com-2024.01.26-15_57_56.webm](https://github.com/elastic/kibana/assets/478762/917b4a76-a48f-4bdc-b3d8-5598f86febf8)


### Checklist

Delete any items that are not applicable to this PR.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Kyle Pollich <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…tion (elastic#175983)

## Summary

- Follow up after elastic#175546
- Part of elastic#165251

introducing a new `is_internal` config option for
`xpack.fleet.fleetServerHosts`. The usage is currently to protect the
internal fleet server hosts in the UI:

- filter them out in the Settings UI
- disable internal hosts in the agent policy form



### Checklist

Delete any items that are not applicable to this PR.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: David Kilfoyle <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related Team:Fleet Team label for Observability Data Collection Fleet team v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants