Commit 066499b8 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch '41054-disable-creation-of-new-kubernetes-integrations' into 'master'

41054-Disallow creation of new Kubernetes integrations

Closes #41054

See merge request gitlab-org/gitlab-ce!16017
parents d9d17a46 e028d795
...@@ -268,3 +268,7 @@ ...@@ -268,3 +268,7 @@
margin: 0 0 5px 17px; margin: 0 0 5px 17px;
} }
} }
.deprecated-service {
cursor: default;
}
...@@ -27,5 +27,16 @@ module ServicesHelper ...@@ -27,5 +27,16 @@ module ServicesHelper
"#{event}_events" "#{event}_events"
end end
def service_save_button(service)
button_tag(class: 'btn btn-save', type: 'submit', disabled: service.deprecated?) do
icon('spinner spin', class: 'hidden js-btn-spinner') +
content_tag(:span, 'Save changes', class: 'js-btn-label')
end
end
def disable_fields_service?(service)
!current_controller?("admin/services") && service.deprecated?
end
extend self extend self
end end
...@@ -31,6 +31,7 @@ class KubernetesService < DeploymentService ...@@ -31,6 +31,7 @@ class KubernetesService < DeploymentService
before_validation :enforce_namespace_to_lower_case before_validation :enforce_namespace_to_lower_case
validate :deprecation_validation, unless: :template?
validates :namespace, validates :namespace,
allow_blank: true, allow_blank: true,
length: 1..63, length: 1..63,
...@@ -145,6 +146,17 @@ class KubernetesService < DeploymentService ...@@ -145,6 +146,17 @@ class KubernetesService < DeploymentService
@kubeclient ||= build_kubeclient! @kubeclient ||= build_kubeclient!
end end
def deprecated?
!active
end
def deprecation_message
content = <<-MESSAGE.strip_heredoc
Kubernetes service integration has been deprecated. #{deprecated_message_content} your clusters using the new <a href=\'#{Gitlab::Routing.url_helpers.project_clusters_path(project)}'/>Clusters</a> page
MESSAGE
content.html_safe
end
TEMPLATE_PLACEHOLDER = 'Kubernetes namespace'.freeze TEMPLATE_PLACEHOLDER = 'Kubernetes namespace'.freeze
private private
...@@ -226,4 +238,20 @@ class KubernetesService < DeploymentService ...@@ -226,4 +238,20 @@ class KubernetesService < DeploymentService
def enforce_namespace_to_lower_case def enforce_namespace_to_lower_case
self.namespace = self.namespace&.downcase self.namespace = self.namespace&.downcase
end end
def deprecation_validation
return if active_changed?(from: true, to: false)
if deprecated?
errors[:base] << deprecation_message
end
end
def deprecated_message_content
if active?
"Your cluster information on this page is still editable, but you are advised to disable and reconfigure"
else
"Fields on this page are now uneditable, you can configure"
end
end
end end
...@@ -263,6 +263,14 @@ class Service < ActiveRecord::Base ...@@ -263,6 +263,14 @@ class Service < ActiveRecord::Base
service service
end end
def deprecated?
false
end
def deprecation_message
nil
end
private private
def cache_project_has_external_issue_tracker def cache_project_has_external_issue_tracker
......
.flash-container.flash-container-page
.flash-alert.deprecated-service
%span= @service.deprecation_message
...@@ -13,10 +13,7 @@ ...@@ -13,10 +13,7 @@
= render 'shared/service_settings', form: form, subject: @service = render 'shared/service_settings', form: form, subject: @service
- if @service.editable? - if @service.editable?
.footer-block.row-content-block .footer-block.row-content-block
%button.btn.btn-save{ type: 'submit' } = service_save_button(@service)
= icon('spinner spin', class: 'hidden js-btn-spinner')
%span.js-btn-label
Save changes
&nbsp; &nbsp;
- if @service.valid? && @service.activated? - if @service.valid? && @service.activated?
- unless @service.can_test? - unless @service.can_test?
......
...@@ -2,4 +2,6 @@ ...@@ -2,4 +2,6 @@
- page_title @service.title, "Services" - page_title @service.title, "Services"
- add_to_breadcrumbs("Settings", edit_project_path(@project)) - add_to_breadcrumbs("Settings", edit_project_path(@project))
= render 'deprecated_message' if @service.deprecation_message
= render 'form' = render 'form'
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
- choices = field[:choices] - choices = field[:choices]
- default_choice = field[:default_choice] - default_choice = field[:default_choice]
- help = field[:help] - help = field[:help]
- disabled = disable_fields_service?(@service)
.form-group .form-group
- if type == "password" && value.present? - if type == "password" && value.present?
...@@ -15,14 +16,14 @@ ...@@ -15,14 +16,14 @@
= form.label name, title, class: "control-label" = form.label name, title, class: "control-label"
.col-sm-10 .col-sm-10
- if type == 'text' - if type == 'text'
= form.text_field name, class: "form-control", placeholder: placeholder, required: required = form.text_field name, class: "form-control", placeholder: placeholder, required: required, disabled: disabled
- elsif type == 'textarea' - elsif type == 'textarea'
= form.text_area name, rows: 5, class: "form-control", placeholder: placeholder, required: required = form.text_area name, rows: 5, class: "form-control", placeholder: placeholder, required: required, disabled: disabled
- elsif type == 'checkbox' - elsif type == 'checkbox'
= form.check_box name = form.check_box name, disabled: disabled
- elsif type == 'select' - elsif type == 'select'
= form.select name, options_for_select(choices, value ? value : default_choice), {}, { class: "form-control" } = form.select name, options_for_select(choices, value ? value : default_choice), {}, { class: "form-control", disabled: disabled}
- elsif type == 'password' - elsif type == 'password'
= form.password_field name, autocomplete: "new-password", class: "form-control", required: value.blank? && :required = form.password_field name, autocomplete: "new-password", class: "form-control", required: value.blank? && required, disabled: disabled
- if help - if help
%span.help-block= help %span.help-block= help
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
.form-group .form-group
= form.label :active, "Active", class: "control-label" = form.label :active, "Active", class: "control-label"
.col-sm-10 .col-sm-10
= form.check_box :active = form.check_box :active, disabled: disable_fields_service?(@service)
- if @service.supported_events.present? - if @service.supported_events.present?
.form-group .form-group
......
---
title: Disable creation of new Kubernetes Integrations unless they're active or created
from template
merge_request: 41054
author:
type: added
...@@ -114,5 +114,41 @@ describe Projects::ServicesController do ...@@ -114,5 +114,41 @@ describe Projects::ServicesController do
expect(flash[:notice]).to eq 'HipChat settings saved, but not activated.' expect(flash[:notice]).to eq 'HipChat settings saved, but not activated.'
end end
end end
context 'with a deprecated service' do
let(:service) { create(:kubernetes_service, project: project) }
before do
put :update,
namespace_id: project.namespace, project_id: project, id: service.to_param, service: { namespace: 'updated_namespace' }
end
it 'should not update the service' do
service.reload
expect(service.namespace).not_to eq('updated_namespace')
end
end
end
describe "GET #edit" do
before do
get :edit, namespace_id: project.namespace, project_id: project, id: service_id
end
context 'with approved services' do
let(:service_id) { 'jira' }
it 'should render edit page' do
expect(response).to be_success
end
end
context 'with a deprecated service' do
let(:service_id) { 'kubernetes' }
it 'should render edit page' do
expect(response).to be_success
end
end
end end
end end
...@@ -18,6 +18,7 @@ FactoryBot.define do ...@@ -18,6 +18,7 @@ FactoryBot.define do
factory :kubernetes_service do factory :kubernetes_service do
project project
type 'KubernetesService'
active true active true
properties({ properties({
api_url: 'https://kubernetes.example.com', api_url: 'https://kubernetes.example.com',
......
require 'spec_helper' require 'spec_helper'
feature 'Interchangeability between KubernetesService and Platform::Kubernetes' do feature 'Interchangeability between KubernetesService and Platform::Kubernetes' do
EXCEPT_METHODS = %i[test title description help fields initialize_properties namespace namespace= api_url api_url=].freeze EXCEPT_METHODS = %i[test title description help fields initialize_properties namespace namespace= api_url api_url= deprecated? deprecation_message].freeze
EXCEPT_METHODS_GREP_V = %w[_touched? _changed? _was].freeze EXCEPT_METHODS_GREP_V = %w[_touched? _changed? _was].freeze
it 'Clusters::Platform::Kubernetes covers core interfaces in KubernetesService' do it 'Clusters::Platform::Kubernetes covers core interfaces in KubernetesService' do
......
...@@ -52,12 +52,75 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do ...@@ -52,12 +52,75 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do
context 'when service is inactive' do context 'when service is inactive' do
before do before do
subject.project = project
subject.active = false subject.active = false
end end
it { is_expected.not_to validate_presence_of(:api_url) } it { is_expected.not_to validate_presence_of(:api_url) }
it { is_expected.not_to validate_presence_of(:token) } it { is_expected.not_to validate_presence_of(:token) }
end end
context 'with a deprecated service' do
let(:kubernetes_service) { create(:kubernetes_service) }
before do
kubernetes_service.update_attribute(:active, false)
kubernetes_service.properties[:namespace] = "foo"
end
it 'should not update attributes' do
expect(kubernetes_service.save).to be_falsy
end
it 'should include an error with a deprecation message' do
kubernetes_service.valid?
expect(kubernetes_service.errors[:base].first).to match(/Kubernetes service integration has been deprecated/)
end
end
context 'with a non-deprecated service' do
let(:kubernetes_service) { create(:kubernetes_service) }
it 'should update attributes' do
kubernetes_service.properties[:namespace] = 'foo'
expect(kubernetes_service.save).to be_truthy
end
end
context 'with an active and deprecated service' do
let(:kubernetes_service) { create(:kubernetes_service) }
before do
kubernetes_service.active = false
kubernetes_service.properties[:namespace] = 'foo'
kubernetes_service.save
end
it 'should deactive the service' do
expect(kubernetes_service.active?).to be_falsy
end
it 'should not include a deprecation message as error' do
expect(kubernetes_service.errors.messages.count).to eq(0)
end
it 'should update attributes' do
expect(kubernetes_service.properties[:namespace]).to eq("foo")
end
end
context 'with a template service' do
let(:kubernetes_service) { create(:kubernetes_service, template: true, active: false) }
before do
kubernetes_service.properties[:namespace] = 'foo'
end
it 'should update attributes' do
expect(kubernetes_service.save).to be_truthy
expect(kubernetes_service.properties[:namespace]).to eq('foo')
end
end
end end
describe '#initialize_properties' do describe '#initialize_properties' do
...@@ -318,4 +381,42 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do ...@@ -318,4 +381,42 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do
it { is_expected.to eq(pods: []) } it { is_expected.to eq(pods: []) }
end end
end end
describe "#deprecated?" do
let(:kubernetes_service) { create(:kubernetes_service) }
context 'with an active kubernetes service' do
it 'should return false' do
expect(kubernetes_service.deprecated?).to be_falsy
end
end
context 'with a inactive kubernetes service' do
it 'should return true' do
kubernetes_service.update_attribute(:active, false)
expect(kubernetes_service.deprecated?).to be_truthy
end
end
end
describe "#deprecation_message" do
let(:kubernetes_service) { create(:kubernetes_service) }
it 'should indicate the service is deprecated' do
expect(kubernetes_service.deprecation_message).to match(/Kubernetes service integration has been deprecated/)
end
context 'if the services is active' do
it 'should return a message' do
expect(kubernetes_service.deprecation_message).to match(/Your cluster information on this page is still editable/)
end
end
context 'if the service is not active' do
it 'should return a message' do
kubernetes_service.update_attribute(:active, false)
expect(kubernetes_service.deprecation_message).to match(/Fields on this page are now uneditable/)
end
end
end
end end
...@@ -254,4 +254,22 @@ describe Service do ...@@ -254,4 +254,22 @@ describe Service do
end end
end end
end end
describe "#deprecated?" do
let(:project) { create(:project, :repository) }
it 'should return false by default' do
service = create(:service, project: project)
expect(service.deprecated?).to be_falsy
end
end
describe "#deprecation_message" do
let(:project) { create(:project, :repository) }
it 'should be empty by default' do
service = create(:service, project: project)
expect(service.deprecation_message).to be_nil
end
end
end end
...@@ -53,6 +53,10 @@ describe API::Services do ...@@ -53,6 +53,10 @@ describe API::Services do
describe "DELETE /projects/:id/services/#{service.dasherize}" do describe "DELETE /projects/:id/services/#{service.dasherize}" do
include_context service include_context service
before do
initialize_service(service)
end
it "deletes #{service}" do it "deletes #{service}" do
delete api("/projects/#{project.id}/services/#{dashed_service}", user) delete api("/projects/#{project.id}/services/#{dashed_service}", user)
...@@ -67,9 +71,7 @@ describe API::Services do ...@@ -67,9 +71,7 @@ describe API::Services do
# inject some properties into the service # inject some properties into the service
before do before do
service_object = project.find_or_initialize_service(service) initialize_service(service)
service_object.properties = service_attrs
service_object.save
end end
it 'returns authentication error when unauthenticated' do it 'returns authentication error when unauthenticated' do
......
...@@ -10,6 +10,10 @@ describe API::V3::Services do ...@@ -10,6 +10,10 @@ describe API::V3::Services do
describe "DELETE /projects/:id/services/#{service.dasherize}" do describe "DELETE /projects/:id/services/#{service.dasherize}" do
include_context service include_context service
before do
initialize_service(service)
end
it "deletes #{service}" do it "deletes #{service}" do
delete v3_api("/projects/#{project.id}/services/#{dashed_service}", user) delete v3_api("/projects/#{project.id}/services/#{dashed_service}", user)
......
...@@ -29,5 +29,13 @@ Service.available_services_names.each do |service| ...@@ -29,5 +29,13 @@ Service.available_services_names.each do |service|
end end
end end
end end
def initialize_service(service)
service_item = project.find_or_initialize_service(service)
service_item.properties = service_attrs
service_item.active = true if service == "kubernetes"
service_item.save
service_item
end
end end
end end
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