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
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7692cfe
add is_internal config option for outputs
maxcold Jan 25, 2024
df81f0a
add e2e test and disable internal outputs in the edit agent policy se…
maxcold Jan 25, 2024
aaedec8
add is_internal flag to kbn-check-mappings-update-cli
maxcold Jan 26, 2024
a3c170f
[CI] Auto-commit changed files from 'node scripts/check_mappings_upda…
kibanamachine Jan 26, 2024
53cb801
fix so integration test
maxcold Jan 26, 2024
8766ecf
fix isInternal checkin useOutputOptions hook
maxcold Jan 26, 2024
ea0c40d
add test for useOutputOptions to validate internal output handling
maxcold Jan 26, 2024
9fb0ff5
add is_internal description for fleet settings docs
maxcold Jan 26, 2024
63984ec
Merge branch 'main' into csp-hide-internal-output-and-fleet-server-host
maxcold Jan 26, 2024
806259b
Update docs wording for is_internal flag
maxcold Jan 26, 2024
c135ba2
Merge branch 'main' into csp-hide-internal-output-and-fleet-server-host
maxcold Jan 29, 2024
229731c
generated bundeled.json for fleet openapi docs
maxcold Jan 29, 2024
ef15d35
update preconfigured output in case is_internal changed
maxcold Jan 29, 2024
327cdda
Merge branch 'main' into csp-hide-internal-output-and-fleet-server-host
maxcold Jan 29, 2024
1f21d1b
add new SO model version with is_internal mapping_additions definition
maxcold Jan 29, 2024
6c043f8
Merge branch 'main' into csp-hide-internal-output-and-fleet-server-host
maxcold Jan 31, 2024
cbc4c05
fix the is_internal.type on the mapping model version to be boolean
maxcold Jan 31, 2024
4ef8151
[CI] Auto-commit changed files from 'node scripts/jest_integration -u…
kibanamachine Jan 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/settings/fleet-settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ NOTE: The `xpack.fleet.outputs` settings are intended for advanced configuration
If `true`, the output specified in `xpack.fleet.outputs` will be the one used to send agent data unless there is another one configured specifically for the agent policy.
`is_default_monitoring`:::
If `true`, the output specified in `xpack.fleet.outputs` will be the one used to send agent monitoring data unless there is another one configured specifically for the agent policy.
`is_internal`:::
If `true`, the output specified in `xpack.fleet.outputs` will be protected from user manipulation in the UI.
maxcold marked this conversation as resolved.
Show resolved Hide resolved
`config`:::
Extra config for that output.
`proxy_id`:::
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

"is_preconfigured",
"key",
"name",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1697,6 +1697,10 @@
"type": "boolean",
"index": false
},
"is_internal": {
"type": "boolean",
"index": false
},
"ssl": {
"type": "binary"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
"infrastructure-ui-source": "113182d6895764378dfe7fa9fa027244f3a457c4",
"ingest-agent-policies": "7633e578f60c074f8267bc50ec4763845e431437",
"ingest-download-sources": "279a68147e62e4d8858c09ad1cf03bd5551ce58d",
"ingest-outputs": "e36a25e789f22b4494be728321f4304a040e286b",
"ingest-outputs": "39f71484e4fc30b5b9a261a9affb3044dcd6973d",
"ingest-package-policies": "f4c2767e852b700a8b82678925b86bac08958b43",
"ingest_manager_settings": "91445219e7115ff0c45d1dabd5d614a80b421797",
"inventory-view": "b8683c8e352a286b4aca1ab21003115a4800af83",
Expand Down
14 changes: 14 additions & 0 deletions x-pack/plugins/fleet/common/openapi/bundled.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5121,6 +5121,8 @@ components:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down Expand Up @@ -5189,6 +5191,8 @@ components:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down Expand Up @@ -5337,6 +5341,8 @@ components:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down Expand Up @@ -5407,6 +5413,8 @@ components:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down Expand Up @@ -5450,6 +5458,8 @@ components:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down Expand Up @@ -5520,6 +5530,8 @@ components:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down Expand Up @@ -5654,6 +5666,8 @@ components:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ properties:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ properties:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ properties:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ properties:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ properties:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ properties:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ properties:
type: boolean
is_default_monitoring:
type: boolean
is_internal:
type: boolean
name:
type: string
type:
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/common/types/models/output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export type OutputPreset = 'custom' | 'balanced' | 'throughput' | 'scale' | 'lat
interface NewBaseOutput {
is_default: boolean;
is_default_monitoring: boolean;
is_internal?: boolean;
is_preconfigured?: boolean;
name: string;
type: ValueOf<OutputType>;
Expand Down
30 changes: 30 additions & 0 deletions x-pack/plugins/fleet/cypress/e2e/fleet_settings_outputs.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,4 +567,34 @@ describe('Outputs', () => {
});
});
});

describe('Outputs List', () => {
beforeEach(() => {
cy.intercept('/api/fleet/outputs', {
items: [
{
id: 'fleet-default-output',
name: 'default',
type: 'elasticsearch',
is_default: true,
is_default_monitoring: true,
},
{
id: 'internal-fleet-output',
name: 'internal output',
type: 'elasticsearch',
is_default: false,
is_default_monitoring: false,
is_internal: true,
},
],
});

cy.visit('/app/fleet/settings');
});

it('should not display internal outputs', () => {
cy.getBySel(SETTINGS_OUTPUTS.TABLE).should('not.contain', 'internal output');
});
});
});
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/cypress/screens/fleet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ export const AGENT_BINARY_SOURCES_FLYOUT = {
export const SETTINGS_OUTPUTS = {
EDIT_BTN: 'editOutputBtn',
ADD_BTN: 'addOutputBtn',
TABLE: 'settingsOutputsTable',
NAME_INPUT: 'settingsOutputsFlyout.nameInput',
TYPE_INPUT: 'settingsOutputsFlyout.typeInput',
ADD_HOST_ROW_BTN: 'fleetServerHosts.multiRowInput.addRowButton',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,39 @@ const mockApiCallsWithRemoteESOutputs = (http: MockedFleetStartServices['http'])
});
};

const mockApiCallsWithInternalOutputs = (http: MockedFleetStartServices['http']) => {
http.get.mockImplementation(async (path) => {
if (typeof path !== 'string') {
throw new Error('Invalid request');
}
if (path === '/api/fleet/outputs') {
return {
data: {
items: [
{
id: 'default-output',
name: 'Default',
type: 'elasticsearch',
is_default: true,
is_default_monitoring: true,
},
{
id: 'internal-output',
name: 'Internal',
type: 'elasticsearch',
is_default: false,
is_default_monitoring: false,
is_internal: true,
},
],
},
};
}

return defaultHttpClientGetImplementation(path);
});
};

describe('useOutputOptions', () => {
it('should generate enabled options if the licence is platinium', async () => {
const testRenderer = createFleetTestRendererMock();
Expand Down Expand Up @@ -550,4 +583,56 @@ describe('useOutputOptions', () => {
expect(result.current.monitoringOutputOptions.length).toEqual(2);
expect(result.current.monitoringOutputOptions[1].value).toEqual('remote1');
});

it('should not enable internal outputs', async () => {
const testRenderer = createFleetTestRendererMock();
mockedUseLicence.mockReturnValue({
hasAtLeast: () => true,
} as unknown as LicenseService);
mockApiCallsWithInternalOutputs(testRenderer.startServices.http);
const { result, waitForNextUpdate } = testRenderer.renderHook(() =>
useOutputOptions({} as AgentPolicy)
);
expect(result.current.isLoading).toBeTruthy();

await waitForNextUpdate();
expect(result.current.dataOutputOptions).toMatchInlineSnapshot(`
Array [
Object {
"disabled": false,
"inputDisplay": "Default (currently Default)",
"value": "@@##DEFAULT_SELECT##@@",
},
Object {
"disabled": false,
"inputDisplay": "Default",
"value": "default-output",
},
Object {
"disabled": true,
"inputDisplay": "Internal",
"value": "internal-output",
},
]
`);
expect(result.current.monitoringOutputOptions).toMatchInlineSnapshot(`
Array [
Object {
"disabled": undefined,
"inputDisplay": "Default (currently Default)",
"value": "@@##DEFAULT_SELECT##@@",
},
Object {
"disabled": false,
"inputDisplay": "Default",
"value": "default-output",
},
Object {
"disabled": true,
"inputDisplay": "Internal",
"value": "internal-output",
},
]
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export function useOutputOptions(agentPolicy: Partial<NewAgentPolicy | AgentPoli
getDefaultOutput(defaultOutputName, defaultOutputDisabled, defaultOutputDisabledMessage),
...outputsRequest.data.items.map((item) => {
const isOutputTypeUnsupported = !allowedOutputTypes.includes(item.type);
const isInternalOutput = !!item.is_internal;

return {
value: item.id,
Expand All @@ -116,7 +117,7 @@ export function useOutputOptions(agentPolicy: Partial<NewAgentPolicy | AgentPoli
/>
) : undefined
),
disabled: !isPolicyPerOutputAllowed || isOutputTypeUnsupported,
disabled: !isPolicyPerOutputAllowed || isOutputTypeUnsupported || isInternalOutput,
};
}),
];
Expand All @@ -133,10 +134,12 @@ export function useOutputOptions(agentPolicy: Partial<NewAgentPolicy | AgentPoli
return [
getDefaultOutput(defaultOutputName),
...outputsRequest.data.items.map((item) => {
const isInternalOutput = !!item.is_internal;

return {
value: item.id,
inputDisplay: item.name,
disabled: !isPolicyPerOutputAllowed,
disabled: !isPolicyPerOutputAllowed || isInternalOutput,
};
}),
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,5 +164,5 @@ export const OutputsTable: React.FunctionComponent<OutputsTableProps> = ({
];
}, [deleteOutput, getHref]);

return <EuiBasicTable columns={columns} items={outputs} />;
return <EuiBasicTable columns={columns} items={outputs} data-test-subj="settingsOutputsTable" />;
};
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const SettingsApp = withConfirmModalProvider(() => {
const flyoutContext = useFlyoutContext();

const { outputs, fleetServerHosts, downloadSources, proxies } = useSettingsAppData();
const outputItems = outputs.data?.items.filter((item) => !item.is_internal);

const { deleteOutput } = useDeleteOutput(outputs.resendRequest);
const { deleteDownloadSource } = useDeleteDownloadSource(downloadSources.resendRequest);
Expand Down Expand Up @@ -78,7 +79,7 @@ export const SettingsApp = withConfirmModalProvider(() => {

if (
(outputs.isLoading && outputs.isInitialRequest) ||
!outputs.data?.items ||
!outputItems ||
(fleetServerHosts.isLoading && fleetServerHosts.isInitialRequest) ||
!fleetServerHosts.data?.items ||
(downloadSources.isLoading && downloadSources.isInitialRequest) ||
Expand Down Expand Up @@ -148,7 +149,7 @@ export const SettingsApp = withConfirmModalProvider(() => {
</Route>
<Route path={FLEET_ROUTING_PATHS.settings_edit_outputs}>
{(route: { match: { params: { outputId: string } } }) => {
const output = outputs.data?.items.find((o) => route.match.params.outputId === o.id);
const output = outputItems.find((o) => route.match.params.outputId === o.id);
if (!output) {
return <Redirect to={FLEET_ROUTING_PATHS.settings} />;
}
Expand Down Expand Up @@ -196,7 +197,7 @@ export const SettingsApp = withConfirmModalProvider(() => {
<SettingsPage
deleteFleetProxy={deleteFleetProxy}
proxies={proxies.data.items}
outputs={outputs.data.items}
outputs={outputItems}
fleetServerHosts={fleetServerHosts.data.items}
deleteOutput={deleteOutput}
deleteFleetServerHost={deleteFleetServerHost}
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/fleet/server/saved_objects/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ const getSavedObjectTypes = (): { [key: string]: SavedObjectsType } => ({
config: { type: 'flattened' },
config_yaml: { type: 'text' },
is_preconfigured: { type: 'boolean', index: false },
is_internal: { type: 'boolean', index: false },
ssl: { type: 'binary' },
proxy_id: { type: 'keyword' },
shipper: {
Expand Down Expand Up @@ -699,6 +700,7 @@ export function registerEncryptedSavedObjects(
'ca_trusted_fingerprint',
'config',
'config_yaml',
'is_internal',
'is_preconfigured',
'proxy_id',
'version',
Expand Down
Loading