Commit e2468c61 authored by Vladimir Shushlin's avatar Vladimir Shushlin Committed by Andrew Fontaine

Disallow editing the environment name

When environment gets updated, slug and tier aren't properly updated.

And we actually can't update slug as it's used by deployment
jobs(e.g. AutoDevOps) and should stay the same for the environment.

https://gitlab.com/gitlab-org/gitlab/-/issues/31268

Changelog: fixed
parent eef6f55f
...@@ -18,6 +18,7 @@ export default { ...@@ -18,6 +18,7 @@ export default {
data() { data() {
return { return {
formEnvironment: { formEnvironment: {
id: this.environment.id,
name: this.environment.name, name: this.environment.name,
externalUrl: this.environment.external_url, externalUrl: this.environment.external_url,
}, },
...@@ -33,7 +34,6 @@ export default { ...@@ -33,7 +34,6 @@ export default {
axios axios
.put(this.updateEnvironmentPath, { .put(this.updateEnvironmentPath, {
id: this.environment.id, id: this.environment.id,
name: this.formEnvironment.name,
external_url: this.formEnvironment.externalUrl, external_url: this.formEnvironment.externalUrl,
}) })
.then(({ data: { path } }) => visitUrl(path)) .then(({ data: { path } }) => visitUrl(path))
......
...@@ -39,12 +39,17 @@ export default { ...@@ -39,12 +39,17 @@ export default {
), ),
nameLabel: __('Name'), nameLabel: __('Name'),
nameFeedback: __('This field is required'), nameFeedback: __('This field is required'),
nameDisabledHelp: __("You cannot rename an environment after it's created."),
nameDisabledLinkText: __('How do I rename an environment?'),
urlLabel: __('External URL'), urlLabel: __('External URL'),
urlFeedback: __('The URL should start with http:// or https://'), urlFeedback: __('The URL should start with http:// or https://'),
save: __('Save'), save: __('Save'),
cancel: __('Cancel'), cancel: __('Cancel'),
}, },
helpPagePath: helpPagePath('ci/environments/index.md'), helpPagePath: helpPagePath('ci/environments/index.md'),
renamingDisabledHelpPagePath: helpPagePath('ci/environments/index.md', {
anchor: 'rename-an-environment',
}),
data() { data() {
return { return {
visited: { visited: {
...@@ -54,6 +59,9 @@ export default { ...@@ -54,6 +59,9 @@ export default {
}; };
}, },
computed: { computed: {
isNameDisabled() {
return Boolean(this.environment.id);
},
valid() { valid() {
return { return {
name: this.visited.name && this.environment.name !== '', name: this.visited.name && this.environment.name !== '',
...@@ -102,10 +110,17 @@ export default { ...@@ -102,10 +110,17 @@ export default {
:state="valid.name" :state="valid.name"
:invalid-feedback="$options.i18n.nameFeedback" :invalid-feedback="$options.i18n.nameFeedback"
> >
<template v-if="isNameDisabled" #description>
{{ $options.i18n.nameDisabledHelp }}
<gl-link :href="$options.renamingDisabledHelpPagePath" target="_blank">
{{ $options.i18n.nameDisabledLinkText }}
</gl-link>
</template>
<gl-form-input <gl-form-input
id="environment_name" id="environment_name"
:value="environment.name" :value="environment.name"
:state="valid.name" :state="valid.name"
:disabled="isNameDisabled"
name="environment[name]" name="environment[name]"
required required
@input="onChange({ ...environment, name: $event })" @input="onChange({ ...environment, name: $event })"
......
...@@ -213,8 +213,14 @@ class Projects::EnvironmentsController < Projects::ApplicationController ...@@ -213,8 +213,14 @@ class Projects::EnvironmentsController < Projects::ApplicationController
end end
end end
def allowed_environment_attributes
attributes = [:external_url]
attributes << :name if action_name == "create"
attributes
end
def environment_params def environment_params
params.require(:environment).permit(:name, :external_url) params.require(:environment).permit(allowed_environment_attributes)
end end
def environment def environment
......
...@@ -194,7 +194,7 @@ PUT /projects/:id/environments/:environments_id ...@@ -194,7 +194,7 @@ PUT /projects/:id/environments/:environments_id
| --------------- | ------- | --------------------------------- | ------------------------------- | | --------------- | ------- | --------------------------------- | ------------------------------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user | | `id` | integer/string | yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user |
| `environment_id` | integer | yes | The ID of the environment | | `environment_id` | integer | yes | The ID of the environment |
| `name` | string | no | The new name of the environment | | `name` | string | no | [Deprecated and will be removed in GitLab 15.0](https://gitlab.com/gitlab-org/gitlab/-/issues/338897) |
| `external_url` | string | no | The new `external_url` | | `external_url` | string | no | The new `external_url` |
```shell ```shell
......
...@@ -728,6 +728,19 @@ like [Review Apps](../review_apps/index.md) (`review/*`). ...@@ -728,6 +728,19 @@ like [Review Apps](../review_apps/index.md) (`review/*`).
The most specific spec takes precedence over the other wildcard matching. In this case, The most specific spec takes precedence over the other wildcard matching. In this case,
the `review/feature-1` spec takes precedence over `review/*` and `*` specs. the `review/feature-1` spec takes precedence over `review/*` and `*` specs.
### Rename an environment
> Renaming environments through the UI [was removed in GitLab 14.3](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68550). Renaming environments through the API was deprected and [will be removed in GitLab 15.0](https://gitlab.com/gitlab-org/gitlab/-/issues/338897).
Renaming an environment through the UI is not possible.
Instead, you need to delete the old environment and create a new one:
1. On the top bar, select **Menu > Projects** and find your project.
1. On the left sidebar, select **Deployments > Environments**.
1. Find the environment and stop it.
1. Delete the environment.
1. Create a new environment with your preferred name.
## Related topics ## Related topics
- [Use GitLab CI to deploy to multiple environments (blog post)](https://about.gitlab.com/blog/2021/02/05/ci-deployment-and-environments/) - [Use GitLab CI to deploy to multiple environments (blog post)](https://about.gitlab.com/blog/2021/02/05/ci-deployment-and-environments/)
......
...@@ -58,7 +58,8 @@ module API ...@@ -58,7 +58,8 @@ module API
end end
params do params do
requires :environment_id, type: Integer, desc: 'The environment ID' requires :environment_id, type: Integer, desc: 'The environment ID'
optional :name, type: String, desc: 'The new environment name' # TODO: disallow renaming via the API https://gitlab.com/gitlab-org/gitlab/-/issues/338897
optional :name, type: String, desc: 'DEPRECATED: Renaming environment can lead to errors, this will be removed in 15.0'
optional :external_url, type: String, desc: 'The new URL on which this deployment is viewable' optional :external_url, type: String, desc: 'The new URL on which this deployment is viewable'
optional :slug, absence: { message: "is automatically generated and cannot be changed" } optional :slug, absence: { message: "is automatically generated and cannot be changed" }
end end
......
...@@ -16632,6 +16632,9 @@ msgstr "" ...@@ -16632,6 +16632,9 @@ msgstr ""
msgid "How do I mirror repositories?" msgid "How do I mirror repositories?"
msgstr "" msgstr ""
msgid "How do I rename an environment?"
msgstr ""
msgid "How do I set up a Google Chat webhook?" msgid "How do I set up a Google Chat webhook?"
msgstr "" msgstr ""
...@@ -38257,6 +38260,9 @@ msgstr "" ...@@ -38257,6 +38260,9 @@ msgstr ""
msgid "You cannot play this scheduled pipeline at the moment. Please wait a minute." msgid "You cannot play this scheduled pipeline at the moment. Please wait a minute."
msgstr "" msgstr ""
msgid "You cannot rename an environment after it's created."
msgstr ""
msgid "You cannot write to a read-only secondary GitLab Geo instance. Please use %{link_to_primary_node} instead." msgid "You cannot write to a read-only secondary GitLab Geo instance. Please use %{link_to_primary_node} instead."
msgstr "" msgstr ""
......
...@@ -222,6 +222,16 @@ RSpec.describe Projects::EnvironmentsController do ...@@ -222,6 +222,16 @@ RSpec.describe Projects::EnvironmentsController do
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
end end
context 'when name is passed' do
let(:params) { environment_params.merge(environment: { name: "new name" }) }
it 'ignores name' do
expect do
subject
end.not_to change { environment.reload.name }
end
end
end end
describe 'PATCH #stop' do describe 'PATCH #stop' do
......
...@@ -15,15 +15,12 @@ const DEFAULT_OPTS = { ...@@ -15,15 +15,12 @@ const DEFAULT_OPTS = {
projectEnvironmentsPath: '/projects/environments', projectEnvironmentsPath: '/projects/environments',
updateEnvironmentPath: '/proejcts/environments/1', updateEnvironmentPath: '/proejcts/environments/1',
}, },
propsData: { environment: { name: 'foo', externalUrl: 'https://foo.example.com' } }, propsData: { environment: { id: '0', name: 'foo', external_url: 'https://foo.example.com' } },
}; };
describe('~/environments/components/edit.vue', () => { describe('~/environments/components/edit.vue', () => {
let wrapper; let wrapper;
let mock; let mock;
let name;
let url;
let form;
const createWrapper = (opts = {}) => const createWrapper = (opts = {}) =>
mountExtended(EditEnvironment, { mountExtended(EditEnvironment, {
...@@ -34,9 +31,6 @@ describe('~/environments/components/edit.vue', () => { ...@@ -34,9 +31,6 @@ describe('~/environments/components/edit.vue', () => {
beforeEach(() => { beforeEach(() => {
mock = new MockAdapter(axios); mock = new MockAdapter(axios);
wrapper = createWrapper(); wrapper = createWrapper();
name = wrapper.findByLabelText('Name');
url = wrapper.findByLabelText('External URL');
form = wrapper.findByRole('form', { name: 'Edit environment' });
}); });
afterEach(() => { afterEach(() => {
...@@ -44,19 +38,22 @@ describe('~/environments/components/edit.vue', () => { ...@@ -44,19 +38,22 @@ describe('~/environments/components/edit.vue', () => {
wrapper.destroy(); wrapper.destroy();
}); });
const findNameInput = () => wrapper.findByLabelText('Name');
const findExternalUrlInput = () => wrapper.findByLabelText('External URL');
const findForm = () => wrapper.findByRole('form', { name: 'Edit environment' });
const showsLoading = () => wrapper.find(GlLoadingIcon).exists(); const showsLoading = () => wrapper.find(GlLoadingIcon).exists();
const submitForm = async (expected, response) => { const submitForm = async (expected, response) => {
mock mock
.onPut(DEFAULT_OPTS.provide.updateEnvironmentPath, { .onPut(DEFAULT_OPTS.provide.updateEnvironmentPath, {
name: expected.name,
external_url: expected.url, external_url: expected.url,
id: '0',
}) })
.reply(...response); .reply(...response);
await name.setValue(expected.name); await findExternalUrlInput().setValue(expected.url);
await url.setValue(expected.url);
await form.trigger('submit'); await findForm().trigger('submit');
await waitForPromises(); await waitForPromises();
}; };
...@@ -65,18 +62,8 @@ describe('~/environments/components/edit.vue', () => { ...@@ -65,18 +62,8 @@ describe('~/environments/components/edit.vue', () => {
expect(header.exists()).toBe(true); expect(header.exists()).toBe(true);
}); });
it.each`
input | value
${() => name} | ${'test'}
${() => url} | ${'https://example.org'}
`('it changes the value of the input to $value', async ({ input, value }) => {
await input().setValue(value);
expect(input().element.value).toBe(value);
});
it('shows loader after form is submitted', async () => { it('shows loader after form is submitted', async () => {
const expected = { name: 'test', url: 'https://google.ca' }; const expected = { url: 'https://google.ca' };
expect(showsLoading()).toBe(false); expect(showsLoading()).toBe(false);
...@@ -86,7 +73,7 @@ describe('~/environments/components/edit.vue', () => { ...@@ -86,7 +73,7 @@ describe('~/environments/components/edit.vue', () => {
}); });
it('submits the updated environment on submit', async () => { it('submits the updated environment on submit', async () => {
const expected = { name: 'test', url: 'https://google.ca' }; const expected = { url: 'https://google.ca' };
await submitForm(expected, [200, { path: '/test' }]); await submitForm(expected, [200, { path: '/test' }]);
...@@ -94,11 +81,24 @@ describe('~/environments/components/edit.vue', () => { ...@@ -94,11 +81,24 @@ describe('~/environments/components/edit.vue', () => {
}); });
it('shows errors on error', async () => { it('shows errors on error', async () => {
const expected = { name: 'test', url: 'https://google.ca' }; const expected = { url: 'https://google.ca' };
await submitForm(expected, [400, { message: ['name taken'] }]); await submitForm(expected, [400, { message: ['uh oh!'] }]);
expect(createFlash).toHaveBeenCalledWith({ message: 'name taken' }); expect(createFlash).toHaveBeenCalledWith({ message: 'uh oh!' });
expect(showsLoading()).toBe(false); expect(showsLoading()).toBe(false);
}); });
it('renders a disabled "Name" field', () => {
const nameInput = findNameInput();
expect(nameInput.attributes().disabled).toBe('disabled');
expect(nameInput.element.value).toBe('foo');
});
it('renders an "External URL" field', () => {
const urlInput = findExternalUrlInput();
expect(urlInput.element.value).toBe('https://foo.example.com');
});
}); });
...@@ -102,4 +102,52 @@ describe('~/environments/components/form.vue', () => { ...@@ -102,4 +102,52 @@ describe('~/environments/components/form.vue', () => {
wrapper = createWrapper({ loading: true }); wrapper = createWrapper({ loading: true });
expect(wrapper.findComponent(GlLoadingIcon).exists()).toBe(true); expect(wrapper.findComponent(GlLoadingIcon).exists()).toBe(true);
}); });
describe('when a new environment is being created', () => {
beforeEach(() => {
wrapper = createWrapper({
environment: {
name: '',
externalUrl: '',
},
});
});
it('renders an enabled "Name" field', () => {
const nameInput = wrapper.findByLabelText('Name');
expect(nameInput.attributes().disabled).toBeUndefined();
expect(nameInput.element.value).toBe('');
});
it('renders an "External URL" field', () => {
const urlInput = wrapper.findByLabelText('External URL');
expect(urlInput.element.value).toBe('');
});
});
describe('when an existing environment is being edited', () => {
beforeEach(() => {
wrapper = createWrapper({
environment: {
id: 1,
name: 'test',
externalUrl: 'https://example.com',
},
});
});
it('renders a disabled "Name" field', () => {
const nameInput = wrapper.findByLabelText('Name');
expect(nameInput.attributes().disabled).toBe('disabled');
expect(nameInput.element.value).toBe('test');
});
it('renders an "External URL" field', () => {
const urlInput = wrapper.findByLabelText('External URL');
expect(urlInput.element.value).toBe('https://example.com');
});
});
}); });
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