From a9bdab58d150d3b547f31bcee175b8c048ae10b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9=20Sol=C3=A0?= Date: Thu, 5 Oct 2017 16:41:24 +0200 Subject: [PATCH] api: added a validate endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For now I've implemented the endpoint for namespaces and registries. Moreover, this commit also adds an index endpoint for registries. Signed-off-by: Miquel Sabaté Solà --- .../modules/namespaces/components/new-form.js | 37 ++++-------- .../modules/namespaces/services/namespaces.js | 22 +++---- app/controllers/namespaces_controller.rb | 23 +------- .../namespaces/components/_form.html.slim | 7 +-- app/views/teams/components/_form.html.slim | 2 +- lib/api/entities.rb | 36 +++++++++++ lib/api/root_api.rb | 2 + lib/api/v1/namespaces.rb | 20 +++++++ lib/api/v1/registries.rb | 58 ++++++++++++++++++ spec/api/grape_api/v1/namespaces_spec.rb | 21 +++++++ spec/api/grape_api/v1/registries_spec.rb | 59 +++++++++++++++++++ 11 files changed, 222 insertions(+), 65 deletions(-) create mode 100644 lib/api/v1/registries.rb create mode 100644 spec/api/grape_api/v1/registries_spec.rb diff --git a/app/assets/javascripts/modules/namespaces/components/new-form.js b/app/assets/javascripts/modules/namespaces/components/new-form.js index dfcc6d176..7528ba26d 100644 --- a/app/assets/javascripts/modules/namespaces/components/new-form.js +++ b/app/assets/javascripts/modules/namespaces/components/new-form.js @@ -28,9 +28,12 @@ export default { team: this.teamName || '', }, timeout: { - name: null, + validate: null, team: null, }, + errors: { + name: [], + }, }; }, @@ -64,19 +67,8 @@ export default { namespace: { name: { required, - format(value) { - // extracted from models/namespace.rb - const regexp = /^[a-z0-9]+(?:[._-][a-z0-9]+)*$/; - - // required already taking care of this - if (value === '') { - return true; - } - - return regexp.test(value); - }, - available(value) { - clearTimeout(this.timeout.name); + validate(value) { + clearTimeout(this.timeout.validate); // required already taking care of this if (value === '') { @@ -84,21 +76,16 @@ export default { } return new Promise((resolve) => { - const searchName = () => { - const promise = NamespacesService.existsByName(value); - - promise.then((exists) => { - // leave it for the back-end - if (exists === null) { - resolve(true); - } + const validate = () => { + const promise = NamespacesService.validate(value); - // if exists, invalid - resolve(!exists); + promise.then((data) => { + set(this.errors, 'name', data.messages.name); + resolve(data.valid); }); }; - this.timeout.name = setTimeout(searchName, 1000); + this.timeout.validate = setTimeout(validate, 1000); }); }, }, diff --git a/app/assets/javascripts/modules/namespaces/services/namespaces.js b/app/assets/javascripts/modules/namespaces/services/namespaces.js index 85166a400..f840a505b 100644 --- a/app/assets/javascripts/modules/namespaces/services/namespaces.js +++ b/app/assets/javascripts/modules/namespaces/services/namespaces.js @@ -8,9 +8,9 @@ const customActions = { method: 'GET', url: '/namespaces/typeahead/{teamName}', }, - existsByName: { - method: 'HEAD', - url: '/namespaces', + validate: { + method: 'GET', + url: 'api/v1/namespaces/validate', }, }; @@ -36,16 +36,10 @@ function searchTeam(teamName) { return resource.teamTypeahead({ teamName }); } -function existsByName(name) { - return resource.existsByName({ name }) - .then(() => true) - .catch((response) => { - if (response.status === 404) { - return false; - } - - return null; - }); +function validate(name) { + return resource.validate({ name }) + .then(response => response.data) + .catch(() => null); } function get(id) { @@ -84,5 +78,5 @@ export default { changeVisibility, searchTeam, teamExists, - existsByName, + validate, }; diff --git a/app/controllers/namespaces_controller.rb b/app/controllers/namespaces_controller.rb index ce25f0d7c..764b984c5 100644 --- a/app/controllers/namespaces_controller.rb +++ b/app/controllers/namespaces_controller.rb @@ -7,15 +7,9 @@ class NamespacesController < ApplicationController after_action :verify_policy_scoped, only: :index # GET /namespaces - # GET /namespaces.json def index - # TODO: remove this! - if request.head? - check_namespace_by_name if params[:name] - else - respond_to do |format| - format.html { skip_policy_scope } - end + respond_to do |format| + format.html { skip_policy_scope } end end @@ -113,19 +107,6 @@ def visibility_param value end - # Checks if namespaces exists based on the name parameter. - # Renders an empty response with 200 if exists or 404 otherwise. - def check_namespace_by_name - skip_policy_scope - namespace = Namespace.find_by(name: params[:name]) - - if namespace - head :ok - else - head :not_found - end - end - def set_namespace @namespace = Namespace.find(params[:id]) end diff --git a/app/views/namespaces/components/_form.html.slim b/app/views/namespaces/components/_form.html.slim index 60fe19263..ad8cc65b1 100644 --- a/app/views/namespaces/components/_form.html.slim +++ b/app/views/namespaces/components/_form.html.slim @@ -9,10 +9,9 @@ div ref="form" class="collapse" span.help-block span v-if="!$v.namespace.name.required" | Name can't be blank - span v-if="!$v.namespace.name.format" - | Name can only contain lower case alphanumeric characters, with optional underscores and dashes in the middle - span v-if="!$v.namespace.name.available" - | Name has already been taken + span.error-messages v-if="errors.name && errors.name.length" + span.error-message v-for="(error, index) in errors.name" :key="index" + | Name {{ error }} .form-group.has-feedback :class=="{ 'has-error': $v.namespace.team.$error }" v-if="!teamName" = f.label :team, {class: "control-label col-md-2"} diff --git a/app/views/teams/components/_form.html.slim b/app/views/teams/components/_form.html.slim index 139c2e44e..c4d6ca157 100644 --- a/app/views/teams/components/_form.html.slim +++ b/app/views/teams/components/_form.html.slim @@ -3,7 +3,7 @@ div ref="form" class="collapse" .form-group :class=="{ 'has-error': $v.team.name.$error }" = f.label :name, {class: 'control-label col-md-2'} .col-md-7 - = f.text_field(:name, class: 'form-control', required: true, placeholder: "New team's name", "@input" => "$v.team.name.$touch()", "v-model.trim" => "team.name") + = f.text_field(:name, class: 'form-control', required: true, placeholder: "New team's name".html_safe, "@input" => "$v.team.name.$touch()", "v-model.trim" => "team.name") span.help-block span v-if="!$v.team.name.required" | Name can't be blank diff --git a/lib/api/entities.rb b/lib/api/entities.rb index a5e96946d..e9ec85f6d 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -12,6 +12,19 @@ class Messages < Grape::Entity expose :message end + # Messages for /validate calls. + class Status < Grape::Entity + expose :messages, documentation: { + type: Hash, + desc: "Detailed hash with the fields" + } + + expose :valid, documentation: { + type: "Boolean", + desc: "Whether the given resource is valid or not" + } + end + # Users and application tokens class Users < Grape::Entity @@ -33,6 +46,29 @@ class ApplicationTokens < Grape::Entity expose :plain_token, if: { type: :create } end + # Registry + + class Registries < Grape::Entity + expose :id, unless: { type: :create }, documentation: { type: Integer } + expose :name, documentation: { + type: String, + desc: "The name of the registry" + } + expose :hostname, documentation: { + type: String, + desc: "The hostname of the registry" + } + expose :external_hostname, documentation: { + type: String, + desc: "An external hostname of the registry, useful if behind a proxy with a different FQDN" + } + expose :use_ssl, documentation: { + type: ::Grape::API::Boolean, + desc: "Whether the registry uses SSL or not" + } + expose :created_at, :updated_at, documentation: { type: DateTime } + end + # Repositories and tags class Tags < Grape::Entity diff --git a/lib/api/root_api.rb b/lib/api/root_api.rb index 5abd6b8ec..d362912c7 100644 --- a/lib/api/root_api.rb +++ b/lib/api/root_api.rb @@ -3,6 +3,7 @@ require "api/entities" require "api/helpers" require "api/v1/namespaces" +require "api/v1/registries" require "api/v1/repositories" require "api/v1/tags" require "api/v1/teams" @@ -44,6 +45,7 @@ class RootAPI < Grape::API helpers ::API::Helpers mount ::API::V1::Namespaces + mount ::API::V1::Registries mount ::API::V1::Repositories mount ::API::V1::Tags mount ::API::V1::Teams diff --git a/lib/api/v1/namespaces.rb b/lib/api/v1/namespaces.rb index 5d79785b9..a22d7ddfa 100644 --- a/lib/api/v1/namespaces.rb +++ b/lib/api/v1/namespaces.rb @@ -27,6 +27,26 @@ class Namespaces < Grape::API type: current_type end + desc "Validates the given namespace", + tags: ["namespaces"], + detail: "Validates the given namespace.", + entity: API::Entities::Status, + failure: [ + [401, "Authentication fails."], + [403, "Authorization fails."] + ] + + params do + requires :name, type: String, documentation: { desc: "Name to be checked." } + end + + get "/validate" do + namespace = Namespace.new(name: params[:name], registry: Registry.get) + valid = namespace.valid? + obj = { valid: valid, messages: namespace.errors.messages } + present obj, with: API::Entities::Status + end + desc "Create a namespace", entity: API::Entities::Teams, failure: [ diff --git a/lib/api/v1/registries.rb b/lib/api/v1/registries.rb new file mode 100644 index 000000000..a381c18fe --- /dev/null +++ b/lib/api/v1/registries.rb @@ -0,0 +1,58 @@ +module API + module V1 + class Registries < Grape::API + version "v1", using: :path + + resource :registries do + before do + authorization!(force_admin: true) + end + + desc "Returns a list of registries", + tags: ["registries"], + detail: "This will expose all accessible registries.", + is_array: true, + entity: API::Entities::Registries, + failure: [ + [401, "Authentication fails."], + [403, "Authorization fails."] + ] + + get do + present Registry.all, with: API::Entities::Registries + end + + desc "Validates the given registry", + tags: ["registries"], + detail: "Besides containing the usual Status object, it adds a `reachable` " \ + "field in the `messages` hash. This field is a string containing the error " \ + "as given by the registry. If empty then everything went well", + entity: API::Entities::Status, + failure: [ + [401, "Authentication fails."], + [403, "Authorization fails."] + ] + + params do + requires :name, + using: API::Entities::Registries.documentation.slice(:name) + requires :hostname, + using: API::Entities::Registries.documentation.slice(:hostname) + optional :external_hostname, + using: API::Entities::Registries.documentation.slice(:external_hostname) + requires :use_ssl, + using: API::Entities::Registries.documentation.slice(:use_ssl) + end + + get "/validate" do + r = Registry.new(permitted_params) + valid = r.valid? + reachable = r.reachable? + fields = r.errors.messages.merge(reachable: reachable) + obj = { valid: valid && reachable, messages: fields } + present obj, with: API::Entities::Status + end + end + end + end +end diff --git a/spec/api/grape_api/v1/namespaces_spec.rb b/spec/api/grape_api/v1/namespaces_spec.rb index 6a91dacf2..7e76d058a 100644 --- a/spec/api/grape_api/v1/namespaces_spec.rb +++ b/spec/api/grape_api/v1/namespaces_spec.rb @@ -222,4 +222,25 @@ expect(namespace_creation_activity.trackable).to eq(namespace) end end + + context "GET /api/v1/namespaces/validate" do + it "returns the proper response when the namespace exists" do + ns = create(:namespace, visibility: public_visibility, team: team) + + get "/api/v1/namespaces/validate", { name: ns.name }, @admin_header + expect(response).to have_http_status(:success) + + data = JSON.parse(response.body) + expect(data["valid"]).to be_falsey + expect(data["messages"]["name"]).to eq(["has already been taken"]) + end + + it "returns the proper response when the namespace does not exist" do + get "/api/v1/namespaces/validate", { name: "somename" }, @admin_header + expect(response).to have_http_status(:success) + + data = JSON.parse(response.body) + expect(data["valid"]).to be_truthy + end + end end diff --git a/spec/api/grape_api/v1/registries_spec.rb b/spec/api/grape_api/v1/registries_spec.rb new file mode 100644 index 000000000..fc7ba0c08 --- /dev/null +++ b/spec/api/grape_api/v1/registries_spec.rb @@ -0,0 +1,59 @@ +require "rails_helper" + +describe API::V1::Registries do + before :each do + admin = create :admin + token = create :application_token, user: admin + @header = build_token_header(token) + end + + context "GET /api/v1/registries" do + it "returns list of registries" do + r = create :registry + get "/api/v1/registries", nil, @header + expect(response).to have_http_status(:success) + + data = JSON.parse(response.body) + expect(data.first["name"]).to eq(r.name) + end + end + + context "GET /api/v1/registries/validate" do + let(:registry_data) do + { + name: "registry", + hostname: "my.registry.cat", + use_ssl: true + } + end + + it "returns valid on a proper registry" do + allow_any_instance_of(Registry).to receive(:reachable?).and_return(true) + get "/api/v1/registries/validate", registry_data, @header + + data = JSON.parse(response.body) + expect(data["messages"]["reachable"]).to be_truthy + expect(data["valid"]).to be_truthy + end + + it "returns unreachable accordingly" do + allow_any_instance_of(Registry).to receive(:reachable?).and_return(false) + get "/api/v1/registries/validate", registry_data, @header + + data = JSON.parse(response.body) + expect(data["messages"]["reachable"]).to be_falsey + expect(data["valid"]).to be_falsey + end + + it "returns an error when the name is already taken" do + create :registry, name: registry_data[:name] + + allow_any_instance_of(Registry).to receive(:reachable?).and_return(true) + get "/api/v1/registries/validate", registry_data, @header + + data = JSON.parse(response.body) + expect(data["messages"]["name"]).to eq(["has already been taken"]) + expect(data["valid"]).to be_falsey + end + end +end