Skip to content

Commit

Permalink
[Fleet] Cascade agent policy's namespace to package policies (elastic…
Browse files Browse the repository at this point in the history
…#174776)

## Summary

Cascade agent policy's namespace to package policies. This change
introduces a new behavior for package policies namespace, mirroring the
way outputs work:
- Allow leaving namespace blank when creating/updating a package policy.
Agent policies namespace keeps being mandatory.
- If a package policy has a namespace defined, that's used to compile
the inputs in the full agent policy.
- If a package policy has blank namespace, use the agent policy one.
- Namespace "default" is no longer used as default for package policy
namespace across the code
- All the checks for the namespace are now moved to full agent policy
handler.

## UI
The only change in the UI is that the namespace in the policy creation
can now be left blank:
![Screenshot 2024-01-16 at 16 55
07](https://github.com/elastic/kibana/assets/16084106/ed1be524-c4e6-49ac-8b53-39b07240d204)
I think that the text is still relevant, as nothing has changed from
that point of view.

## Testing
### Creation - Inherit namespace
- Create agent policy with namespace `agentpolicyspace`
- Create package policy with blank namespace
- Generate full agent policy; package policy should have namespace
`agentpolicyspace`

### Creation - Package policy custom namespace
- Create agent policy with namespace `agentpolicyspace`
- Create package policy with namespace `packagepolicyspace`
- Generate full agent policy; package policy should have namespace
`packagepolicyspace`

### Update - Change default namespace
- Create agent policy with namespace  `agentpolicyspace`
- Create package policy with blank namespace
- Generate full agent policy; Package policy should have namespace
`agentpolicyspace`
- Change agent policy namespace to `newspace`
- Package policy should have namespace `newspace` as well

### Update - Don't override custom namespace
- Create agent policy with namespace `agentpolicyspace`
- Create package policy with namespace `packagepolicyspace`
- Change agent policy namespace to `newspace`

### Check full agent policy
- namespace should be correct; check inputs for the package policies and
output permissions as well

### Checklist
- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [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
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
criamico and kibanamachine authored Jan 18, 2024
1 parent 1a51e61 commit 4e41582
Show file tree
Hide file tree
Showing 28 changed files with 416 additions and 73 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/fleet/common/openapi/bundled.json
Original file line number Diff line number Diff line change
Expand Up @@ -7671,8 +7671,8 @@
},
"namespace": {
"type": "string",
"description": "namespace by default \"default\"",
"example": "default"
"description": "The package policy namespace. Leave blank to inherit the agent policy's namespace.",
"example": "customnamespace"
},
"policy_id": {
"type": "string",
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/fleet/common/openapi/bundled.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4932,8 +4932,10 @@ components:
example: my description
namespace:
type: string
description: namespace by default "default"
example: default
description: >-
The package policy namespace. Leave blank to inherit the agent
policy's namespace.
example: customnamespace
policy_id:
type: string
description: Agent policy ID where that package policy will be added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ properties:
example: 'my description'
namespace:
type: string
description: namespace by default "default"
example: 'default'
description: The package policy namespace. Leave blank to inherit the agent policy's namespace.
example: 'customnamespace'
policy_id:
type: string
description: Agent policy ID where that package policy will be added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,5 @@ properties:
type: boolean
required:
- name
- namespace
- policy_id
- enabled
7 changes: 5 additions & 2 deletions x-pack/plugins/fleet/common/services/is_valid_namespace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ import { i18n } from '@kbn/i18n';
// Namespace string eventually becomes part of an index name. This method partially implements index name rules from
// https://github.com/elastic/elasticsearch/blob/master/docs/reference/indices/create-index.asciidoc
// and implements a limit based on https://github.com/elastic/kibana/issues/75846
export function isValidNamespace(namespace: string): { valid: boolean; error?: string } {
if (!namespace.trim()) {
export function isValidNamespace(
namespace: string,
allowBlankNamespace?: boolean
): { valid: boolean; error?: string } {
if (!namespace.trim() && !allowBlankNamespace) {
return {
valid: false,
error: i18n.translate('xpack.fleet.namespaceValidation.requiredErrorMessage', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ export const validatePackagePolicy = (
inputs: {},
vars: {},
};
const namespaceValidation = isValidNamespace(packagePolicy.namespace);

if (!packagePolicy.name.trim()) {
validationResults.name = [
i18n.translate('xpack.fleet.packagePolicyValidation.nameRequiredErrorMessage', {
Expand All @@ -73,8 +71,11 @@ export const validatePackagePolicy = (
];
}

if (!namespaceValidation.valid && namespaceValidation.error) {
validationResults.namespace = [namespaceValidation.error];
if (packagePolicy?.namespace) {
const namespaceValidation = isValidNamespace(packagePolicy?.namespace, true);
if (!namespaceValidation.valid && namespaceValidation.error) {
validationResults.namespace = [namespaceValidation.error];
}
}

// Validate package-level vars
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/fleet/common/types/models/package_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export interface NewPackagePolicy {
id?: string | number;
name: string;
description?: string;
namespace: string;
namespace?: string;
enabled: boolean;
is_managed?: boolean;
policy_id: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ export const StepDefinePackagePolicy: React.FunctionComponent<{
>
<EuiComboBox
noSuggestions
placeholder={agentPolicy?.namespace}
isDisabled={isEditPage && packageInfo.type === 'input'}
singleSelection={true}
selectedOptions={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export const AddIntegrationPageStep: React.FC<MultiPageStepLayoutProps> = (props
const [packagePolicy, setPackagePolicy] = useState<NewPackagePolicy>({
name: '',
description: '',
namespace: 'default',
namespace: '',
policy_id: '',
enabled: true,
inputs: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const sendGeneratePackagePolicy = async (
const defaultPolicy: NewPackagePolicy = {
name: incrementedName,
description: '',
namespace: 'default',
namespace: '',
policy_id: agentPolicyId,
enabled: true,
inputs: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { RenderHookResult } from '@testing-library/react-hooks';

import type { TestRenderer } from '../../../../../../../mock';
import { createFleetTestRendererMock } from '../../../../../../../mock';
import type { AgentPolicy, PackageInfo } from '../../../../../types';
import type { PackageInfo } from '../../../../../types';

import { sendGetPackagePolicies } from '../../../../../hooks';

Expand Down Expand Up @@ -78,7 +78,7 @@ describe('useOnSubmit', () => {
packageInfo,
withSysMonitoring: false,
selectedPolicyTab: SelectedPolicyTab.NEW,
newAgentPolicy: { name: 'test', namespace: 'default' },
newAgentPolicy: { name: 'test', namespace: '' },
queryParamsPolicyId: undefined,
})
));
Expand All @@ -94,21 +94,23 @@ describe('useOnSubmit', () => {
});
});

it('should set package policy id and namespace when agent policy changes', () => {
it('should set new values when package policy changes', () => {
act(() => {
renderResult.result.current.updateAgentPolicy({
id: 'some-id',
namespace: 'default',
} as AgentPolicy);
renderResult.result.current.updatePackagePolicy({
id: 'new-id',
namespace: 'newspace',
name: 'apache-2',
});
});

expect(renderResult.result.current.packagePolicy).toEqual({
policy_id: 'some-id',
namespace: 'default',
id: 'new-id',
policy_id: '',
namespace: 'newspace',
description: '',
enabled: true,
inputs: [],
name: 'apache-1',
name: 'apache-2',
package: {
name: 'apache',
title: 'Apache',
Expand Down Expand Up @@ -142,7 +144,7 @@ describe('useOnSubmit', () => {
enabled: true,
inputs: [],
name: 'apache-1',
namespace: 'default',
namespace: '',
policy_id: '',
package: {
name: 'apache',
Expand Down Expand Up @@ -187,7 +189,7 @@ describe('useOnSubmit', () => {
enabled: true,
inputs: [],
name: 'apache-11',
namespace: 'default',
namespace: '',
policy_id: '',
package: {
name: 'apache',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ async function savePackagePolicy(pkgPolicy: CreatePackagePolicyRequest['body'])
const DEFAULT_PACKAGE_POLICY = {
name: '',
description: '',
namespace: 'default',
namespace: '',
policy_id: '',
enabled: true,
inputs: [],
Expand Down Expand Up @@ -221,7 +221,7 @@ export function useOnSubmit({
packageToPackagePolicy(
packageInfo,
agentPolicy?.id || '',
agentPolicy?.namespace || DEFAULT_PACKAGE_POLICY.namespace,
'',
DEFAULT_PACKAGE_POLICY.name || incrementedName,
DEFAULT_PACKAGE_POLICY.description,
integrationToEnable
Expand All @@ -236,7 +236,6 @@ export function useOnSubmit({
if (agentPolicy && packagePolicy.policy_id !== agentPolicy.id) {
updatePackagePolicy({
policy_id: agentPolicy.id,
namespace: agentPolicy.namespace,
});
}
}, [packagePolicy, agentPolicy, updatePackagePolicy]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ describe('when on the package policy create page', () => {
},
],
name: 'nginx-1',
namespace: 'default',
namespace: '',
package: {
name: 'nginx',
title: 'Nginx',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
EuiIcon,
EuiToolTip,
EuiLink,
EuiIconTip,
} from '@elastic/eui';

import { INTEGRATIONS_PLUGIN_ID } from '../../../../../../../../common';
Expand Down Expand Up @@ -79,7 +80,6 @@ export const PackagePoliciesTable: React.FunctionComponent<Props> = ({
if (packagePolicy.namespace) {
namespacesValues.add(packagePolicy.namespace);
}

const hasUpgrade = isPackagePolicyUpgradable(packagePolicy);

return {
Expand All @@ -95,7 +95,6 @@ export const PackagePoliciesTable: React.FunctionComponent<Props> = ({
const namespaceFilterOptions = [...namespacesValues]
.sort(stringSortAscending)
.map(toFilterOption);

return [mappedPackagePolicies, namespaceFilterOptions];
}, [originalPackagePolicies, isPackagePolicyUpgradable]);

Expand Down Expand Up @@ -225,7 +224,19 @@ export const PackagePoliciesTable: React.FunctionComponent<Props> = ({
}
),
render: (namespace: InMemoryPackagePolicy['namespace']) => {
return namespace ? <EuiBadge color="hollow">{namespace}</EuiBadge> : '';
return namespace ? (
<EuiBadge color="hollow">{namespace}</EuiBadge>
) : (
<>
<EuiBadge color="default">{agentPolicy.namespace}</EuiBadge>
<EuiIconTip
content="Namespace defined in parent agent policy"
position="right"
type="iInCircle"
color="subdued"
/>
</>
);
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ export const updatePackagePolicyHandler: FleetRequestHandler<
...body,
name: body.name ?? packagePolicy.name,
description: body.description ?? packagePolicy.description,
namespace: body.namespace ?? packagePolicy.namespace,
namespace: body.namespace ?? packagePolicy?.namespace,
policy_id: body.policy_id ?? packagePolicy.policy_id,
enabled: 'enabled' in body ? body.enabled ?? packagePolicy.enabled : packagePolicy.enabled,
package: pkg ?? packagePolicy.package,
Expand Down
Loading

0 comments on commit 4e41582

Please sign in to comment.