From 2cb1d617d90b4a9311e3a35434bec958f266d22a Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 2 Oct 2017 17:13:46 +0900 Subject: Use expires_in for access_token validation --- app/controllers/google_api/authorizations_controller.rb | 10 +++++++--- app/controllers/projects/clusters_controller.rb | 16 +++++++++++----- lib/google_api/auth.rb | 3 ++- lib/google_api/cloud_platform/client.rb | 16 ++++++++++++++++ 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/app/controllers/google_api/authorizations_controller.rb b/app/controllers/google_api/authorizations_controller.rb index 00b0c128711..890b4ce60c8 100644 --- a/app/controllers/google_api/authorizations_controller.rb +++ b/app/controllers/google_api/authorizations_controller.rb @@ -1,9 +1,13 @@ module GoogleApi class AuthorizationsController < ApplicationController def callback - session[GoogleApi::CloudPlatform::Client.session_key_for_token] = - GoogleApi::CloudPlatform::Client.new(nil, callback_google_api_authorizations_url) - .get_token(params[:code]) + token, expires_at = GoogleApi::CloudPlatform::Client + .new(nil, callback_google_api_authorizations_url) + .get_token(params[:code]) + + session[GoogleApi::CloudPlatform::Client.session_key_for_token] = token + session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = + expires_at.to_s if params[:state] redirect_to params[:state] diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index ebb17bca010..552cc48d84a 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -6,12 +6,11 @@ class Projects::ClustersController < Projects::ApplicationController def login begin @authorize_url = GoogleApi::CloudPlatform::Client.new( - nil, - callback_google_api_authorizations_url, + nil, callback_google_api_authorizations_url, state: namespace_project_clusters_url.to_s ).authorize_url rescue GoogleApi::Auth::ConfigMissingError - # Show an alert message that gitlab.yml is not configured properly + # no-op end end @@ -83,12 +82,19 @@ class Projects::ClustersController < Projects::ApplicationController end def authorize_google_api - unless token_in_session + unless GoogleApi::CloudPlatform::Client.new(token_in_session, nil) + .validate_token(expires_at_in_session) redirect_to action: 'login' end end def token_in_session - @token_in_session ||= session[GoogleApi::CloudPlatform::Client.session_key_for_token] + @token_in_session ||= + session[GoogleApi::CloudPlatform::Client.session_key_for_token] + end + + def expires_at_in_session + @expires_at_in_session ||= + session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] end end diff --git a/lib/google_api/auth.rb b/lib/google_api/auth.rb index 92787b87ac6..8c962af51d7 100644 --- a/lib/google_api/auth.rb +++ b/lib/google_api/auth.rb @@ -19,7 +19,8 @@ module GoogleApi end def get_token(code) - client.auth_code.get_token(code, redirect_uri: redirect_uri).token + ret = client.auth_code.get_token(code, redirect_uri: redirect_uri) + return ret.token, ret.expires_at end protected diff --git a/lib/google_api/cloud_platform/client.rb b/lib/google_api/cloud_platform/client.rb index a1abc5bf074..ec77e6bdd72 100644 --- a/lib/google_api/cloud_platform/client.rb +++ b/lib/google_api/cloud_platform/client.rb @@ -9,12 +9,28 @@ module GoogleApi def session_key_for_token :cloud_platform_access_token end + + def session_key_for_expires_at + :cloud_platform_expires_at + end end def scope 'https://www.googleapis.com/auth/cloud-platform' end + def validate_token(expires_at) + return false unless access_token + return false unless expires_at + + # Making sure that the token will have been still alive during the cluster creation. + unless DateTime.strptime(expires_at, '%s').to_time > Time.now + 10.minutes + return false + end + + true + end + def projects_zones_clusters_get(project_id, zone, cluster_id) service = Google::Apis::ContainerV1::ContainerService.new service.authorization = access_token -- cgit v1.2.1 From 34e66c427dde2070c2c09a07ce08f991e46de92f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 2 Oct 2017 21:58:50 +0900 Subject: PollingInterval, rename to gke_clusters, has_one :cluster --- app/controllers/projects/clusters_controller.rb | 10 ++- app/models/ci/cluster.rb | 78 -------------------- app/models/gcp/cluster.rb | 93 ++++++++++++++++++++++++ app/models/project.rb | 2 +- app/services/ci/create_cluster_service.rb | 4 +- app/services/ci/integrate_cluster_service.rb | 19 +---- app/services/ci/update_cluster_service.rb | 4 +- app/workers/cluster_creation_worker.rb | 2 +- app/workers/wait_for_cluster_creation_worker.rb | 4 +- db/migrate/20170924094327_create_ci_clusters.rb | 48 ------------ db/migrate/20170924094327_create_gcp_clusters.rb | 44 +++++++++++ db/schema.rb | 37 +++++++++- lib/gitlab/gcp/model.rb | 13 ++++ lib/google_api/cloud_platform/client.rb | 2 +- 14 files changed, 204 insertions(+), 156 deletions(-) delete mode 100644 app/models/ci/cluster.rb create mode 100644 app/models/gcp/cluster.rb delete mode 100644 db/migrate/20170924094327_create_ci_clusters.rb create mode 100644 db/migrate/20170924094327_create_gcp_clusters.rb create mode 100644 lib/gitlab/gcp/model.rb diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index 552cc48d84a..2c53e034428 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -15,15 +15,15 @@ class Projects::ClustersController < Projects::ApplicationController end def index - if project.clusters.any? - redirect_to edit_project_cluster_path(project, project.clusters.last.id) + if project.cluster + redirect_to edit_project_cluster_path(project, project.cluster) else redirect_to new_project_cluster_path(project) end end def new - @cluster = project.clusters.new + @cluster = project.build_cluster end def create @@ -42,6 +42,8 @@ class Projects::ClustersController < Projects::ApplicationController def status respond_to do |format| format.json do + Gitlab::PollingInterval.set_header(response, interval: 10_000) + render json: { status: cluster.status, # The current status of the operation. status_reason: cluster.status_reason # If an error has occurred, a textual description of the error. @@ -72,7 +74,7 @@ class Projects::ClustersController < Projects::ApplicationController private def cluster - @cluster ||= project.clusters.find(params[:id]) + @cluster ||= project.cluster end def cluster_params diff --git a/app/models/ci/cluster.rb b/app/models/ci/cluster.rb deleted file mode 100644 index 93f582f565a..00000000000 --- a/app/models/ci/cluster.rb +++ /dev/null @@ -1,78 +0,0 @@ -module Ci - class Cluster < ActiveRecord::Base - extend Gitlab::Ci::Model - - belongs_to :project - belongs_to :user - belongs_to :service - - attr_encrypted :password, - mode: :per_attribute_iv_and_salt, - insecure_mode: true, - key: Gitlab::Application.secrets.db_key_base, - algorithm: 'aes-256-cbc' - - attr_encrypted :kubernetes_token, - mode: :per_attribute_iv_and_salt, - insecure_mode: true, - key: Gitlab::Application.secrets.db_key_base, - algorithm: 'aes-256-cbc' - - attr_encrypted :gcp_token, - mode: :per_attribute_iv_and_salt, - insecure_mode: true, - key: Gitlab::Application.secrets.db_key_base, - algorithm: 'aes-256-cbc' - - enum status: { - unknown: nil, - scheduled: 1, - creating: 2, - created: 3, - errored: 4 - } - - validates :gcp_project_id, presence: true - validates :cluster_zone, presence: true - validates :cluster_name, presence: true - validates :cluster_size, presence: true, - numericality: { only_integer: true, greater_than: 0 } - validate :restrict_modification, on: :update - - def errored!(reason) - self.status = :errored - self.status_reason = reason - self.gcp_token = nil - - save!(validate: false) - end - - def creating!(gcp_operation_id) - self.status = :creating - self.gcp_operation_id = gcp_operation_id - - save!(validate: false) - end - - def on_creation? - scheduled? || creating? - end - - def api_url - 'https://' + endpoint - end - - def restrict_modification - if on_creation? - errors.add(:base, "cannot modify during creation") - return false - end - - true - end - - def destroy - super if restrict_modification - end - end -end diff --git a/app/models/gcp/cluster.rb b/app/models/gcp/cluster.rb new file mode 100644 index 00000000000..006cb1feb0c --- /dev/null +++ b/app/models/gcp/cluster.rb @@ -0,0 +1,93 @@ +module Gcp + class Cluster < ActiveRecord::Base + extend Gitlab::Gcp::Model + + belongs_to :project, inverse_of: :cluster + belongs_to :user + belongs_to :service + + attr_encrypted :password, + mode: :per_attribute_iv, + insecure_mode: true, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' + + attr_encrypted :kubernetes_token, + mode: :per_attribute_iv, + insecure_mode: true, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' + + attr_encrypted :gcp_token, + mode: :per_attribute_iv, + insecure_mode: true, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' + + enum status: { + unknown: nil, + scheduled: 1, + creating: 2, + created: 3, + errored: 4 + } + + validates :gcp_project_id, presence: true + validates :cluster_zone, presence: true + validates :cluster_name, presence: true + validates :cluster_size, presence: true, + numericality: { only_integer: true, greater_than: 0 } + validate :restrict_modification, on: :update + + def errored!(reason) + self.status = :errored + self.status_reason = reason + self.gcp_token = nil + + save!(validate: false) + end + + def creating!(gcp_operation_id) + self.status = :creating + self.gcp_operation_id = gcp_operation_id + + save!(validate: false) + end + + def created!(endpoint, ca_cert, kubernetes_token, username, password) + self.status = :created + self.enabled = true + self.endpoint = endpoint + self.ca_cert = ca_cert + self.kubernetes_token = kubernetes_token + self.username = username + self.password = password + self.service = project.find_or_initialize_service('kubernetes') + self.gcp_token = nil + self.gcp_operation_id = nil + + save! + end + + def on_creation? + scheduled? || creating? + end + + def api_url + 'https://' + endpoint + end + + def restrict_modification + if on_creation? + errors.add(:base, "cannot modify during creation") + return false + end + + true + end + + def destroy + super if restrict_modification + end + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 6b896746864..5d6a8bdbaeb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -163,6 +163,7 @@ class Project < ActiveRecord::Base has_one :import_data, class_name: 'ProjectImportData', inverse_of: :project, autosave: true has_one :project_feature, inverse_of: :project has_one :statistics, class_name: 'ProjectStatistics' + has_one :cluster, class_name: 'Gcp::Cluster', inverse_of: :project # Container repositories need to remove data from the container registry, # which is not managed by the DB. Hence we're still using dependent: :destroy @@ -171,7 +172,6 @@ class Project < ActiveRecord::Base has_many :commit_statuses has_many :pipelines, class_name: 'Ci::Pipeline' - has_many :clusters, class_name: 'Ci::Cluster' # Ci::Build objects store data on the file system such as artifact files and # build traces. Currently there's no efficient way of removing this data in diff --git a/app/services/ci/create_cluster_service.rb b/app/services/ci/create_cluster_service.rb index b5fb71f5092..48d7c9aef23 100644 --- a/app/services/ci/create_cluster_service.rb +++ b/app/services/ci/create_cluster_service.rb @@ -5,9 +5,9 @@ module Ci params['machine_type'] = GoogleApi::CloudPlatform::Client::DEFAULT_MACHINE_TYPE end - project.clusters.create( + project.create_cluster( params.merge(user: current_user, - status: Ci::Cluster.statuses[:scheduled], + status: Gcp::Cluster.statuses[:scheduled], gcp_token: access_token)) end end diff --git a/app/services/ci/integrate_cluster_service.rb b/app/services/ci/integrate_cluster_service.rb index 5dd1f2b3414..c60d3722373 100644 --- a/app/services/ci/integrate_cluster_service.rb +++ b/app/services/ci/integrate_cluster_service.rb @@ -1,23 +1,10 @@ module Ci class IntegrateClusterService def execute(cluster, endpoint, ca_cert, token, username, password) - Ci::Cluster.transaction do - kubernetes_service ||= - cluster.project.find_or_initialize_service('kubernetes') + Gcp::Cluster.transaction do + cluster.created!(endpoint, ca_cert, token, username, password) - cluster.update!( - enabled: true, - service: kubernetes_service, - username: username, - password: password, - kubernetes_token: token, - ca_cert: ca_cert, - endpoint: endpoint, - gcp_token: nil, - gcp_operation_id: nil, - status: Ci::Cluster.statuses[:created]) - - kubernetes_service.update!( + cluster.service.update!( active: true, api_url: cluster.api_url, ca_pem: ca_cert, diff --git a/app/services/ci/update_cluster_service.rb b/app/services/ci/update_cluster_service.rb index a440ac03a0b..a517d6be0c7 100644 --- a/app/services/ci/update_cluster_service.rb +++ b/app/services/ci/update_cluster_service.rb @@ -1,7 +1,7 @@ module Ci class UpdateClusterService < BaseService def execute(cluster) - Ci::Cluster.transaction do + Gcp::Cluster.transaction do cluster.update!(enabled: params['enabled']) if params['enabled'] == 'true' @@ -12,7 +12,7 @@ module Ci namespace: cluster.project_namespace, token: cluster.kubernetes_token) else - cluster.service.update(active: false) + cluster.service.update!(active: false) end end rescue ActiveRecord::RecordInvalid => e diff --git a/app/workers/cluster_creation_worker.rb b/app/workers/cluster_creation_worker.rb index 40e40005022..697cb73c2ac 100644 --- a/app/workers/cluster_creation_worker.rb +++ b/app/workers/cluster_creation_worker.rb @@ -3,7 +3,7 @@ class ClusterCreationWorker include DedicatedSidekiqQueue def perform(cluster_id) - cluster = Ci::Cluster.find_by_id(cluster_id) + cluster = Gcp::Cluster.find_by_id(cluster_id) unless cluster return Rails.logger.error "Cluster object is not found; #{cluster_id}" diff --git a/app/workers/wait_for_cluster_creation_worker.rb b/app/workers/wait_for_cluster_creation_worker.rb index 5ffe70d4ba6..0dd60de5150 100644 --- a/app/workers/wait_for_cluster_creation_worker.rb +++ b/app/workers/wait_for_cluster_creation_worker.rb @@ -7,7 +7,7 @@ class WaitForClusterCreationWorker TIMEOUT = 20.minutes def perform(cluster_id) - cluster = Ci::Cluster.find_by_id(cluster_id) + cluster = Gcp::Cluster.find_by_id(cluster_id) unless cluster return Rails.logger.error "Cluster object is not found; #{cluster_id}" @@ -56,7 +56,7 @@ class WaitForClusterCreationWorker username = gke_cluster.master_auth.username password = gke_cluster.master_auth.password rescue Exception => e - return cluster.errored!("Can not extract the extected data; #{e}") + return cluster.errored!("Can not extract the expected data; #{e}") end kubernetes_token = Ci::FetchKubernetesTokenService.new( diff --git a/db/migrate/20170924094327_create_ci_clusters.rb b/db/migrate/20170924094327_create_ci_clusters.rb deleted file mode 100644 index bef39b9808d..00000000000 --- a/db/migrate/20170924094327_create_ci_clusters.rb +++ /dev/null @@ -1,48 +0,0 @@ -class CreateCiClusters < ActiveRecord::Migration - DOWNTIME = false - - def up - create_table :ci_clusters do |t| - t.references :project, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade } - t.references :user, null: false, foreign_key: true - t.references :service, foreign_key: true - - # General - t.boolean :enabled, default: true - t.integer :status - t.string :status_reason - - # k8s integration specific - t.string :project_namespace - - # Cluster details - t.string :endpoint - t.text :ca_cert - t.string :encrypted_kubernetes_token - t.string :encrypted_kubernetes_token_salt - t.string :encrypted_kubernetes_token_iv - t.string :username - t.string :encrypted_password - t.string :encrypted_password_salt - t.string :encrypted_password_iv - - # GKE - t.string :gcp_project_id, null: false - t.string :cluster_zone, null: false - t.string :cluster_name, null: false - t.integer :cluster_size, null: false - t.string :machine_type - t.string :gcp_operation_id - t.string :encrypted_gcp_token - t.string :encrypted_gcp_token_salt - t.string :encrypted_gcp_token_iv - - t.datetime_with_timezone :created_at, null: false - t.datetime_with_timezone :updated_at, null: false - end - end - - def down - drop_table :ci_clusters - end -end diff --git a/db/migrate/20170924094327_create_gcp_clusters.rb b/db/migrate/20170924094327_create_gcp_clusters.rb new file mode 100644 index 00000000000..9aa8e537dbe --- /dev/null +++ b/db/migrate/20170924094327_create_gcp_clusters.rb @@ -0,0 +1,44 @@ +class CreateGcpClusters < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :gcp_clusters do |t| + t.references :project, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade } + t.references :user, null: false, foreign_key: true + t.references :service, foreign_key: true + + # General + t.boolean :enabled, default: true + t.integer :status + t.string :status_reason + + # k8s integration specific + t.string :project_namespace + + # Cluster details + t.string :endpoint + t.text :ca_cert + t.string :encrypted_kubernetes_token + t.string :encrypted_kubernetes_token_salt + t.string :encrypted_kubernetes_token_iv + t.string :username + t.string :encrypted_password + t.string :encrypted_password_salt + t.string :encrypted_password_iv + + # GKE + t.string :gcp_project_id, null: false + t.string :cluster_zone, null: false + t.string :cluster_name, null: false + t.integer :cluster_size, null: false + t.string :machine_type + t.string :gcp_operation_id + t.string :encrypted_gcp_token + t.string :encrypted_gcp_token_salt + t.string :encrypted_gcp_token_iv + + t.datetime_with_timezone :created_at, null: false + t.datetime_with_timezone :updated_at, null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 61245f3f666..62802cd20ee 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170924094327) do +ActiveRecord::Schema.define(version: 20170928100231) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -608,6 +608,38 @@ ActiveRecord::Schema.define(version: 20170924094327) do add_index "forked_project_links", ["forked_to_project_id"], name: "index_forked_project_links_on_forked_to_project_id", unique: true, using: :btree + create_table "gcp_clusters", force: :cascade do |t| + t.integer "project_id", null: false + t.integer "user_id", null: false + t.integer "service_id" + t.boolean "enabled", default: true + t.integer "status" + t.string "status_reason" + t.string "project_namespace" + t.string "endpoint" + t.text "ca_cert" + t.string "encrypted_kubernetes_token" + t.string "encrypted_kubernetes_token_salt" + t.string "encrypted_kubernetes_token_iv" + t.string "username" + t.string "encrypted_password" + t.string "encrypted_password_salt" + t.string "encrypted_password_iv" + t.string "gcp_project_id", null: false + t.string "cluster_zone", null: false + t.string "cluster_name", null: false + t.integer "cluster_size", null: false + t.string "machine_type" + t.string "gcp_operation_id" + t.string "encrypted_gcp_token" + t.string "encrypted_gcp_token_salt" + t.string "encrypted_gcp_token_iv" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "gcp_clusters", ["project_id"], name: "index_gcp_clusters_on_project_id", unique: true, using: :btree + create_table "gpg_keys", force: :cascade do |t| t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false @@ -1742,6 +1774,9 @@ ActiveRecord::Schema.define(version: 20170924094327) do add_foreign_key "events", "projects", on_delete: :cascade add_foreign_key "events", "users", column: "author_id", name: "fk_edfd187b6f", on_delete: :cascade add_foreign_key "forked_project_links", "projects", column: "forked_to_project_id", name: "fk_434510edb0", on_delete: :cascade + add_foreign_key "gcp_clusters", "projects", on_delete: :cascade + add_foreign_key "gcp_clusters", "services" + add_foreign_key "gcp_clusters", "users" add_foreign_key "gpg_keys", "users", on_delete: :cascade add_foreign_key "gpg_signatures", "gpg_keys", on_delete: :nullify add_foreign_key "gpg_signatures", "projects", on_delete: :cascade diff --git a/lib/gitlab/gcp/model.rb b/lib/gitlab/gcp/model.rb new file mode 100644 index 00000000000..195391f0e3c --- /dev/null +++ b/lib/gitlab/gcp/model.rb @@ -0,0 +1,13 @@ +module Gitlab + module Gcp + module Model + def table_name_prefix + "gcp_" + end + + def model_name + @model_name ||= ActiveModel::Name.new(self, nil, self.name.split("::").last) + end + end + end +end diff --git a/lib/google_api/cloud_platform/client.rb b/lib/google_api/cloud_platform/client.rb index ec77e6bdd72..aa85fcdabef 100644 --- a/lib/google_api/cloud_platform/client.rb +++ b/lib/google_api/cloud_platform/client.rb @@ -86,7 +86,7 @@ module GoogleApi end def parse_operation_id(self_link) - self_link.match(/projects\/.*\/zones\/.*\/operations\/(.*)/)[1] + self_link.match(%r{projects/.*/zones/.*/operations/(.*)})[1] end end end -- cgit v1.2.1