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.

Sign up for GitHub

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] Azure integration manual fields #171069

Merged
merged 14 commits into from
Nov 29, 2023
Merged

[Cloud Security] Azure integration manual fields #171069

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2f4c8e0
init added fields, waiting for integration pr to be merged
JordanSh Nov 9, 2023
7c8290f
add all fields, waiting for integration pr to be merged
JordanSh Nov 12, 2023
537963a
before adding field clearing
JordanSh Nov 15, 2023
a25c4a8
added tests, need to add field clearing and finalize titles and infos
JordanSh Nov 16, 2023
2b20d42
Merge branch 'main' of https://github.com/elastic/kibana into azure-m…
JordanSh Nov 16, 2023
b12d7c7
added field clearing
JordanSh Nov 16, 2023
16f79d5
added version rendering of new ui
JordanSh Nov 16, 2023
1eb7b37
added test
JordanSh Nov 16, 2023
46732af
clean
JordanSh Nov 16, 2023
fac3fc8
fixed default value
JordanSh Nov 23, 2023
f5adfaf
fixed default value
JordanSh Nov 23, 2023
a3b3b64
removed unneeded test
JordanSh Nov 23, 2023
05fab2a
Merge branch 'main' into azure-manual-fields
kibanamachine Nov 28, 2023
3acdeaa
Merge branch 'main' into azure-manual-fields
jeniawhite Nov 29, 2023
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
54 changes: 1 addition & 53 deletions x-pack/plugins/cloud_security_posture/common/utils/helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,7 @@
*/

import { createPackagePolicyMock } from '@kbn/fleet-plugin/common/mocks';
import {
getBenchmarkFromPackagePolicy,
getBenchmarkFilter,
cleanupCredentials,
mapCredentialsOptionsToFieldMap,
} from './helpers';
import { getBenchmarkFromPackagePolicy, getBenchmarkFilter, cleanupCredentials } from './helpers';

describe('test helper methods', () => {
it('get default integration type from inputs with multiple enabled types', () => {
Expand Down Expand Up @@ -301,50 +296,3 @@ describe('test helper methods', () => {
});
});
});

describe('mapCredentialsOptionsToFieldMap', () => {
it('should return an empty object when given an empty object as input', () => {
const options = {};
const result = mapCredentialsOptionsToFieldMap(options);
expect(result).toEqual({});
});

it('should return a field map with the same keys as the input object', () => {
const options = {
option1: { fields: {} },
option2: { fields: {} },
option3: { fields: {} },
};
const result = mapCredentialsOptionsToFieldMap(options);
expect(Object.keys(result)).toEqual(Object.keys(options));
});

it('should return a field map with the correct fields for each option', () => {
const options = {
option1: { fields: { field1: 'value1', field2: 'value2' } },
option2: { fields: { field3: 'value3', field4: 'value4' } },
option3: { fields: { field5: 'value5', field6: 'value6', field7: 'value7' } },
};
const result = mapCredentialsOptionsToFieldMap(options);
expect(result).toEqual({
option1: ['field1', 'field2'],
option2: ['field3', 'field4'],
option3: ['field5', 'field6', 'field7'],
});
});

it('should return an empty object when given null as input', () => {
const options = null as any;
const result = mapCredentialsOptionsToFieldMap(options);
expect(result).toEqual({});
});

it('should return an empty object when given an object with null or undefined fields as input', () => {
const options = {
option1: { fields: null },
option2: { fields: undefined },
} as any;
const result = mapCredentialsOptionsToFieldMap(options);
expect(result).toEqual({});
});
});
12 changes: 12 additions & 0 deletions ...ture/public/components/fleet_extensions/azure_credentials_form/azure_credentials_form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,18 @@ const TemporaryManualSetup = ({ integrationLink }: { integrationLink: string })
const AZURE_MINIMUM_PACKAGE_VERSION = '1.6.0';
const AZURE_MANUAL_FIELDS_PACKAGE_VERSION = '1.7.0';

export const getDefaultAzureManualCredentialType = (packageInfo: PackageInfo) => {
const packageSemanticVersion = semverValid(packageInfo.version);
const cleanPackageVersion = semverCoerce(packageSemanticVersion) || '';

const isPackageVersionValidForManualFields = !semverLt(
cleanPackageVersion,
AZURE_MANUAL_FIELDS_PACKAGE_VERSION
);

return isPackageVersionValidForManualFields ? 'managed_identity' : 'manual';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I find it a bit confusing that when "package is valid for manual" we return managed identity otherwise manual. without the context, I would think it should be the other way around. Maybe change to isPackageVersionValidForManagedIdentity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

umm this is a very good point, thing is that "manual fields" are meant to reference the entire options (fields) under the Manual credential type, managed identity is just one of them and it happens to be the default value out of those options.

do you think something like isPackageVersionValidForManualOption is better? i dont mind sticking with your suggestions, just wanted to give more context and see what do you think

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I see where the confusion is coming from, that we have Manual as a term that includes multiple setup methods, and inside it we have another Manual as its own setup method, as in "really manual, you need to do everything yourself" kind of thing. changing to isPackageVersionValidForManualOption tbh wouldn't change much for me, in the end starting from 1.7.0 we started to support Manual -> Managed Identity so it became the default, and before it was Manual -> Manual the default one. So isPackageVersionValidForManagedIdentity still makes more sense to me (even though it's not only Managed Identity but some other options we started to support) but I'm also ok with a comment in this function to explain this Manual extravaganza, without changing the name :)

};

const AzureInputVarFields = ({
fields,
onChange,
Expand Down
34 changes: 24 additions & 10 deletions ...components/fleet_extensions/azure_credentials_form/get_azure_credentials_form_options.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import { NewPackagePolicyInput } from '@kbn/fleet-plugin/common';
import React from 'react';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import { EuiText } from '@elastic/eui';
import { AzureCredentialsType } from '../../../../common/types';

export type AzureCredentialsFields = Record<string, { label: string; type?: 'password' | 'text' }>;
Expand Down Expand Up @@ -45,8 +47,6 @@ export const getInputVarsFields = (input: NewPackagePolicyInput, fields: AzureCr
} as const;
});

export const DEFAULT_AZURE_MANUAL_CREDENTIALS_TYPE = 'service_principal_with_client_secret';

const I18N_TENANT_ID = i18n.translate('xpack.csp.azureIntegration.tenantIdLabel', {
defaultMessage: 'Tenant ID',
});
Expand All @@ -56,6 +56,20 @@ const I18N_CLIENT_ID = i18n.translate('xpack.csp.azureIntegration.clientIdLabel'
});

export const getAzureCredentialsFormOptions = (): AzureOptions => ({
managed_identity: {
label: i18n.translate('xpack.csp.azureIntegration.credentialType.managedIdentityLabel', {
defaultMessage: 'Managed Identity',
}),
info: (
<EuiText color="subdued" size="s">
<FormattedMessage
id="xpack.csp.azureIntegration.credentialType.managedIdentityInfo"
defaultMessage="Ensure the agent is deployed on a resource that supports managed identities (e.g., Azure Virtual Machines). No explicit credentials need to be provided; Azure handles the authentication."
/>
</EuiText>
),
fields: {},
},
arm_template: {
label: 'ARM Template',
info: [],
Expand Down Expand Up @@ -117,18 +131,18 @@ export const getAzureCredentialsFormOptions = (): AzureOptions => ({
},
},
},
managed_identity: {
label: i18n.translate('xpack.csp.azureIntegration.credentialType.managedIdentityLabel', {
defaultMessage: 'Managed Identity',
}),
info: [],
fields: {},
},
manual: {
label: i18n.translate('xpack.csp.azureIntegration.credentialType.manualLabel', {
defaultMessage: 'Manual',
}),
info: [],
info: (
<EuiText color="subdued" size="s">
<FormattedMessage
id="xpack.csp.azureIntegration.credentialType.manualInfo"
defaultMessage="Ensure the agent is deployed on a resource that supports managed identities (e.g., Azure Virtual Machines). No explicit credentials need to be provided; Azure handles the authentication."
/>
</EuiText>
),
fields: {},
},
});
10 changes: 7 additions & 3 deletions ...cloud_security_posture/public/components/fleet_extensions/azure_credentials_form/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@

import { useEffect, useRef } from 'react';
import { NewPackagePolicy, PackageInfo } from '@kbn/fleet-plugin/common';
import { AZURE_ARM_TEMPLATE_CREDENTIAL_TYPE } from './azure_credentials_form';
import {
AZURE_ARM_TEMPLATE_CREDENTIAL_TYPE,
getDefaultAzureManualCredentialType,
} from './azure_credentials_form';
import { cspIntegrationDocsNavigation } from '../../../common/navigation/constants';
import {
getArmTemplateUrlFromCspmPackage,
getPosturePolicy,
NewPackagePolicyPostureInput,
} from '../utils';
import {
DEFAULT_AZURE_MANUAL_CREDENTIALS_TYPE,
getAzureCredentialsFormOptions,
getInputVarsFields,
} from './get_azure_credentials_form_options';
Expand Down Expand Up @@ -149,6 +151,8 @@ export const useAzureCredentialsForm = ({
setupFormat,
});

const defaultAzureManualCredentialType = getDefaultAzureManualCredentialType(packageInfo);

const onSetupFormatChange = (newSetupFormat: SetupFormat) => {
if (newSetupFormat === AZURE_ARM_TEMPLATE_CREDENTIAL_TYPE) {
fieldsSnapshot.current = Object.fromEntries(
Expand All @@ -170,7 +174,7 @@ export const useAzureCredentialsForm = ({
updatePolicy(
getPosturePolicy(newPolicy, input.type, {
'azure.credentials.type': {
value: lastManualCredentialsType.current || DEFAULT_AZURE_MANUAL_CREDENTIALS_TYPE,
value: lastManualCredentialsType.current || defaultAzureManualCredentialType,
type: 'text',
},
...fieldsSnapshot.current,
Expand Down