Commit 111ae858 authored by ap4y's avatar ap4y

Implement edit flow in policy editor

This commit implements support for policy editing in policy
editor. Editing is implemented as a standalone page using existing
components.
parent 998ece3e
import initPolicyEditorApp from 'ee/threat_monitoring/policy_editor';
document.addEventListener('DOMContentLoaded', initPolicyEditorApp);
......@@ -12,7 +12,7 @@ import {
} from '@gitlab/ui';
import { s__ } from '~/locale';
import { getTimeago } from '~/lib/utils/datetime_utility';
import { setUrlFragment } from '~/lib/utils/url_utility';
import { setUrlFragment, mergeUrlParams } from '~/lib/utils/url_utility';
import EnvironmentPicker from './environment_picker.vue';
import NetworkPolicyEditor from './network_policy_editor.vue';
import PolicyDrawer from './policy_editor/policy_drawer.vue';
......@@ -74,14 +74,29 @@ export default {
hasAutoDevopsPolicy() {
return this.policiesWithDefaults.some(policy => policy.isAutodevops);
},
hasCiliumSelectedPolicy() {
return this.hasSelectedPolicy
? this.selectedPolicy.manifest.includes(CiliumNetworkPolicyKind)
: false;
},
shouldShowCiliumDrawer() {
if (!this.hasSelectedPolicy) return false;
return this.glFeatures.networkPolicyEditor && this.hasCiliumSelectedPolicy;
},
shouldShowEditButton() {
return (
this.glFeatures.networkPolicyEditor &&
this.selectedPolicy.manifest.includes(CiliumNetworkPolicyKind)
this.hasCiliumSelectedPolicy &&
Boolean(this.selectedPolicy.creationTimestamp)
);
},
editPolicyPath() {
return this.hasSelectedPolicy
? mergeUrlParams(
{ environment_id: this.currentEnvironmentId },
this.newPolicyPath.replace('new', `${this.selectedPolicyName}/edit`),
)
: '';
},
},
methods: {
...mapActions('networkPolicies', ['createPolicy', 'updatePolicy']),
......@@ -230,6 +245,14 @@ export default {
<h3 class="gl-mb-3">{{ selectedPolicy.name }}</h3>
<div>
<gl-button ref="cancelButton" @click="deselectPolicy">{{ __('Cancel') }}</gl-button>
<gl-button
v-if="shouldShowEditButton"
data-testid="edit-button"
category="primary"
variant="info"
:href="editPolicyPath"
>{{ s__('NetworkPolicies|Edit policy') }}</gl-button
>
<gl-button
ref="applyButton"
category="primary"
......
......@@ -113,6 +113,7 @@ function parseRule(item, direction) {
*/
export default function fromYaml(manifest) {
const { description, metadata, spec } = safeLoad(manifest, { json: true });
const { name, resourceVersion } = metadata;
const { endpointSelector = {}, ingress = [], egress = [] } = spec;
const matchLabels = endpointSelector.matchLabels || {};
......@@ -130,7 +131,8 @@ export default function fromYaml(manifest) {
.filter(rule => Boolean(rule));
return {
name: metadata.name,
name,
resourceVersion,
description,
isEnabled: !Object.keys(matchLabels).includes(DisabledByLabel),
endpointMatchMode: endpointLabels.length > 0 ? EndpointMatchModeLabel : EndpointMatchModeAny,
......
......@@ -33,7 +33,11 @@ function spec({ rules, isEnabled, endpointMatchMode, endpointLabels }) {
Return yaml representation of a policy.
*/
export default function toYaml(policy) {
const { name, description } = policy;
const { name, resourceVersion, description } = policy;
const metadata = { name };
if (resourceVersion) {
metadata.resourceVersion = resourceVersion;
}
const policySpec = {
apiVersion: 'cilium.io/v2',
......@@ -46,7 +50,7 @@ export default function toYaml(policy) {
// We want description at a specific position to have yaml in a common form.
Object.assign(policySpec, {
metadata: { name },
metadata,
spec: spec(policy),
});
......
......@@ -49,20 +49,29 @@ export default {
type: String,
required: true,
},
existingPolicy: {
type: Object,
required: false,
default: null,
},
},
data() {
return {
editorMode: EditorModeRule,
yamlEditorValue: '',
yamlEditorError: null,
policy: {
const policy = this.existingPolicy
? fromYaml(this.existingPolicy.manifest)
: {
name: '',
description: '',
isEnabled: false,
endpointMatchMode: EndpointMatchModeAny,
endpointLabels: '',
rules: [],
},
};
return {
editorMode: EditorModeRule,
yamlEditorValue: '',
yamlEditorError: null,
policy,
};
},
computed: {
......@@ -83,13 +92,21 @@ export default {
hasParsingError() {
return Boolean(this.yamlEditorError);
},
isEditing() {
return Boolean(this.existingPolicy);
},
saveButtonText() {
return this.isEditing
? s__('NetworkPolicies|Save changes')
: s__('NetworkPolicies|Create policy');
},
},
created() {
this.fetchEnvironments();
},
methods: {
...mapActions('threatMonitoring', ['fetchEnvironments']),
...mapActions('networkPolicies', ['createPolicy']),
...mapActions('networkPolicies', ['createPolicy', 'updatePolicy']),
addRule() {
this.policy.rules.push(buildRule(RuleTypeEndpoint));
},
......@@ -121,11 +138,13 @@ export default {
this.editorMode = mode;
},
savePolicy() {
const saveFn = this.isEditing ? this.updatePolicy : this.createPolicy;
const policy = { manifest: toYaml(this.policy) };
return this.createPolicy({
environmentId: this.currentEnvironmentId,
policy,
}).then(() => {
if (this.isEditing) {
policy.name = this.existingPolicy.name;
}
return saveFn({ environmentId: this.currentEnvironmentId, policy }).then(() => {
if (!this.errorUpdatingPolicy) redirectTo(this.threatMonitoringPath);
});
},
......@@ -262,9 +281,9 @@ export default {
type="submit"
category="primary"
variant="success"
data-testid="create-policy"
data-testid="save-policy"
@click="savePolicy"
>{{ s__('NetworkPolicies|Create policy') }}</gl-button
>{{ saveButtonText }}</gl-button
>
<gl-button category="secondary" variant="default" :href="threatMonitoringPath">{{
__('Cancel')
......
......@@ -4,7 +4,12 @@ import createStore from './store';
export default () => {
const el = document.querySelector('#js-policy-builder-app');
const { environmentsEndpoint, networkPoliciesEndpoint, threatMonitoringPath } = el.dataset;
const {
environmentsEndpoint,
networkPoliciesEndpoint,
threatMonitoringPath,
policy,
} = el.dataset;
const store = createStore();
store.dispatch('threatMonitoring/setEndpoints', {
......@@ -14,13 +19,16 @@ export default () => {
networkPoliciesEndpoint,
});
const props = { threatMonitoringPath };
if (policy) {
props.existingPolicy = JSON.parse(policy);
}
return new Vue({
el,
store,
render(createElement) {
return createElement(PolicyEditorApp, {
props: { threatMonitoringPath },
});
return createElement(PolicyEditorApp, { props });
},
});
};
......@@ -169,6 +169,7 @@ module EE
projects/licenses#index
projects/threat_monitoring#show
projects/threat_monitoring#new
projects/threat_monitoring#edit
]
end
......
......@@ -6,10 +6,10 @@ exports[`NetworkPolicyList component renders policies table 1`] = `
<table
aria-busy="false"
aria-colcount="3"
aria-describedby="__BVID__334__caption_"
aria-describedby="__BVID__529__caption_"
aria-multiselectable="false"
class="table b-table gl-table table-hover b-table-stacked-md b-table-selectable b-table-select-single"
id="__BVID__334"
id="__BVID__529"
role="table"
>
<!---->
......
......@@ -29,7 +29,7 @@ describe('NetworkPolicyList component', () => {
wrapper = mount(NetworkPolicyList, {
propsData: {
documentationPath: 'documentation_path',
newPolicyPath: 'new_policy_path',
newPolicyPath: '/policies/new',
...propsData,
},
data,
......@@ -86,7 +86,31 @@ describe('NetworkPolicyList component', () => {
expect(wrapper.find(PolicyDrawer).exists()).toBe(false);
});
it('does not render edit button', () => {
expect(wrapper.find('[data-testid="edit-button"]').exists()).toBe(false);
});
describe('given there is a selected policy', () => {
beforeEach(() => {
factory({
provide: {
glFeatures: {
networkPolicyEditor: true,
},
},
data: () => ({ selectedPolicyName: 'policy' }),
});
});
});
describe('given selected policy is a cilium policy', () => {
const manifest = `apiVersion: cilium.io/v2
kind: CiliumNetworkPolicy
metadata:
name: policy
spec:
endpointSelector: {}`;
beforeEach(() => {
factory({
provide: {
......@@ -94,20 +118,13 @@ describe('NetworkPolicyList component', () => {
networkPolicyEditor: true,
},
},
data: () => ({
selectedPolicyName: 'policy',
}),
data: () => ({ selectedPolicyName: 'policy' }),
state: {
policies: [
{
name: 'policy',
isEnabled: false,
manifest: `apiVersion: cilium.io/v2
kind: CiliumNetworkPolicy
metadata:
name: test-policy
spec:
endpointSelector: {}`,
creationTimestamp: new Date(),
manifest,
},
],
},
......@@ -117,6 +134,12 @@ spec:
it('renders the new policy drawer', () => {
expect(wrapper.find(PolicyDrawer).exists()).toBe(true);
});
it('renders edit button', () => {
const button = wrapper.find('[data-testid="edit-button"]');
expect(button.exists()).toBe(true);
expect(button.attributes().href).toBe('/policies/policy/edit?environment_id=-1');
});
});
});
......
......@@ -217,7 +217,7 @@ spec:
>
<gl-button-stub
category="primary"
data-testid="create-policy"
data-testid="save-policy"
icon=""
size="medium"
type="submit"
......
......@@ -52,6 +52,18 @@ describe('fromYaml', () => {
});
});
describe('when resourceVersion is not empty', () => {
beforeEach(() => {
policy.resourceVersion = '1234';
});
it('returns policy object', () => {
expect(fromYaml(toYaml(policy))).toMatchObject({
resourceVersion: '1234',
});
});
});
describe('when policy is disabled', () => {
beforeEach(() => {
policy.isEnabled = false;
......
......@@ -40,6 +40,25 @@ spec:
});
});
describe('when resourceVersion is not empty', () => {
beforeEach(() => {
policy.resourceVersion = '1234';
});
it('returns yaml representation', () => {
expect(toYaml(policy)).toEqual(`apiVersion: cilium.io/v2
kind: CiliumNetworkPolicy
metadata:
name: test-policy
resourceVersion: '1234'
spec:
endpointSelector:
matchLabels:
network-policy.gitlab.com/disabled_by: gitlab
`);
});
});
describe('when policy is enabled', () => {
beforeEach(() => {
policy.isEnabled = true;
......
......@@ -13,6 +13,7 @@ import {
} from 'ee/threat_monitoring/components/policy_editor/constants';
import fromYaml from 'ee/threat_monitoring/components/policy_editor/lib/from_yaml';
import toYaml from 'ee/threat_monitoring/components/policy_editor/lib/to_yaml';
import { buildRule } from 'ee/threat_monitoring/components/policy_editor/lib/rules';
import { redirectTo } from '~/lib/utils/url_utility';
jest.mock('~/lib/utils/url_utility');
......@@ -48,6 +49,8 @@ describe('PolicyEditorApp component', () => {
const findAddRuleButton = () => wrapper.find('[data-testid="add-rule"]');
const findYAMLParsingAlert = () => wrapper.find('[data-testid="parsing-alert"]');
const findNetworkPolicyEditor = () => wrapper.find(NetworkPolicyEditor);
const findPolicyName = () => wrapper.find("[id='policyName']");
const findSavePolicy = () => wrapper.find("[data-testid='save-policy']");
beforeEach(() => {
factory();
......@@ -126,7 +129,7 @@ spec:
beforeEach(() => {
initialValue = findPreview().props('policyYaml');
wrapper.find("[id='policyName']").vm.$emit('input', 'new');
findPolicyName().vm.$emit('input', 'new');
});
it('updates policy yaml preview', () => {
......@@ -169,7 +172,7 @@ spec:
});
it('updates yaml editor value on switch to yaml editor', async () => {
wrapper.find("[id='policyName']").vm.$emit('input', 'test-policy');
findPolicyName().vm.$emit('input', 'test-policy');
wrapper.find("[data-testid='editor-mode']").vm.$emit('input', EditorModeYAML);
await wrapper.vm.$nextTick();
......@@ -199,7 +202,7 @@ spec:
});
it('creates policy and redirects to a threat monitoring path', async () => {
wrapper.find("[data-testid='create-policy']").vm.$emit('click');
findSavePolicy().vm.$emit('click');
await wrapper.vm.$nextTick();
expect(store.dispatch).toHaveBeenCalledWith('networkPolicies/createPolicy', {
......@@ -219,10 +222,64 @@ spec:
});
it('it does not redirect', async () => {
wrapper.find("[data-testid='create-policy']").vm.$emit('click');
findSavePolicy().vm.$emit('click');
await wrapper.vm.$nextTick();
expect(redirectTo).not.toHaveBeenCalledWith('/threat-monitoring');
});
});
describe('given existingPolicy property was provided', () => {
const manifest = toYaml({
name: 'policy',
endpointLabels: '',
rules: [buildRule()],
});
beforeEach(() => {
factory({
propsData: {
existingPolicy: { name: 'policy', manifest },
},
});
});
it('presents existing policy', () => {
expect(findPolicyName().attributes().value).toEqual('policy');
expect(wrapper.findAll(PolicyRuleBuilder).length).toEqual(1);
});
it('updates existing policy and redirects to a threat monitoring path', async () => {
const saveButton = findSavePolicy();
expect(saveButton.text()).toEqual('Save changes');
saveButton.vm.$emit('click');
await wrapper.vm.$nextTick();
expect(store.dispatch).toHaveBeenCalledWith('networkPolicies/updatePolicy', {
environmentId: -1,
policy: { name: 'policy', manifest: toYaml(wrapper.vm.policy) },
});
expect(redirectTo).toHaveBeenCalledWith('/threat-monitoring');
});
describe('given there is a updatePolicy error', () => {
beforeEach(() => {
factory({
propsData: {
existingPolicy: { name: 'policy', manifest },
},
state: {
errorUpdatingPolicy: true,
},
});
});
it('it does not redirect', async () => {
findSavePolicy().vm.$emit('click');
await wrapper.vm.$nextTick();
expect(redirectTo).not.toHaveBeenCalledWith('/threat-monitoring');
});
});
});
});
......@@ -182,6 +182,7 @@ RSpec.describe ProjectsHelper do
projects/licenses#index
projects/threat_monitoring#show
projects/threat_monitoring#new
projects/threat_monitoring#edit
]
end
......
......@@ -16322,6 +16322,9 @@ msgstr ""
msgid "NetworkPolicies|Description"
msgstr ""
msgid "NetworkPolicies|Edit policy"
msgstr ""
msgid "NetworkPolicies|Editor mode"
msgstr ""
......@@ -16400,6 +16403,9 @@ msgstr ""
msgid "NetworkPolicies|Rules"
msgstr ""
msgid "NetworkPolicies|Save changes"
msgstr ""
msgid "NetworkPolicies|Something went wrong, failed to update policy"
msgstr ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment