From ca8b0657ebfe6c7d51c04c45f978a8fb7bf7e017 Mon Sep 17 00:00:00 2001 From: Luke Meyer Date: Thu, 23 Jan 2014 18:22:44 -0500 Subject: [PATCH 1/3] standardize whitespace --- controller/app/models/application.rb | 36 ++++++++++++++-------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/controller/app/models/application.rb b/controller/app/models/application.rb index bbde6fae624..df07013598e 100644 --- a/controller/app/models/application.rb +++ b/controller/app/models/application.rb @@ -146,7 +146,7 @@ def self.with_gear_counts(domains=queryable) gear_sizes[gear_sz] ||= 0 gear_sizes[gear_sz] += gi.gears.length if gi.gears.present? end if app.group_instances.present? - apps_info << {"_id" => app._id, "domain_id" => app.domain_id, "gear_sizes" => gear_sizes} + apps_info << {"_id" => app._id, "domain_id" => app.domain_id, "gear_sizes" => gear_sizes} end apps_info end @@ -625,7 +625,7 @@ def add_features(features, group_overrides=[], init_git_url=nil, user_env_vars=n result_io = ResultIO.new Application.run_in_application_lock(self) do # save this app before adding the op_group, if its persisted already - # this ensures that any downloaded carts are present before the op_group with the features is added + # this ensures that any downloaded carts are present before the op_group with the features is added self.save! if self.persisted? op_group = AddFeaturesOpGroup.new(features: features.map(&:name), group_overrides: group_overrides, init_git_url: init_git_url, @@ -1503,9 +1503,9 @@ def run_jobs(result_io=nil) op_group.delete rescue Exception => e_orig Rails.logger.error "Encountered error during execute '#{e_orig.message}'" - # don't log the error stacktrace if this exception was raised just to trigger a rollback + # don't log the error stacktrace if this exception was raised just to trigger a rollback Rails.logger.debug e_orig.backtrace.inspect unless rollback_pending - + #rollback begin # reload the application before a rollback @@ -1612,8 +1612,8 @@ def calculate_scale_by(ginst_id, scale_by) def calculate_remove_group_instance_ops(comp_specs, group_instance) pending_ops = [] - gear_destroy_ops = calculate_gear_destroy_ops(group_instance._id.to_s, - group_instance.gears.map{|g| g._id.to_s}, + gear_destroy_ops = calculate_gear_destroy_ops(group_instance._id.to_s, + group_instance.gears.map{|g| g._id.to_s}, group_instance.addtl_fs_gb) pending_ops.push(*gear_destroy_ops) gear_destroy_op_ids = gear_destroy_ops.map{|op| op._id.to_s} @@ -1649,8 +1649,8 @@ def calculate_gear_create_ops(ginst_id, gear_ids, deploy_gear_id, comp_specs, co app_dns_gear_id = gear_id.to_s end - init_gear_op = InitGearOp.new(group_instance_id: ginst_id, gear_id: gear_id, - comp_specs: comp_specs, host_singletons: host_singletons, + init_gear_op = InitGearOp.new(group_instance_id: ginst_id, gear_id: gear_id, + comp_specs: comp_specs, host_singletons: host_singletons, app_dns: app_dns, pre_save: (not self.persisted?)) init_gear_op.prereq << prereq_op._id.to_s unless prereq_op.nil? @@ -1712,8 +1712,8 @@ def calculate_gear_create_ops(ginst_id, gear_ids, deploy_gear_id, comp_specs, co end prereq_op_id = prereq_op._id.to_s rescue nil - ops, usage_ops = calculate_add_component_ops(comp_specs, ginst_id, deploy_gear_id, gear_id_prereqs, component_ops, - is_scale_up, (user_vars_op_id || prereq_op_id), init_git_url, + ops, usage_ops = calculate_add_component_ops(comp_specs, ginst_id, deploy_gear_id, gear_id_prereqs, component_ops, + is_scale_up, (user_vars_op_id || prereq_op_id), init_git_url, app_dns_gear_id) pending_ops.push(*ops) track_usage_ops.push(*usage_ops) @@ -1733,7 +1733,7 @@ def calculate_gear_destroy_ops(ginst_id, gear_ids, additional_filesystem_gb) delete_gear_op = DeleteGearOp.new(gear_id: gear_id, prereq: [unreserve_uid_op._id.to_s]) track_usage_op = TrackUsageOp.new(user_id: self.domain.owner._id, parent_user_id: self.domain.owner.parent_user_id, app_name: self.name, gear_id: gear_id, event: UsageRecord::EVENTS[:end], - usage_type: UsageRecord::USAGE_TYPES[:gear_usage], + usage_type: UsageRecord::USAGE_TYPES[:gear_usage], prereq: [delete_gear_op._id.to_s]) ops = [] @@ -1747,7 +1747,7 @@ def calculate_gear_destroy_ops(ginst_id, gear_ids, additional_filesystem_gb) if additional_filesystem_gb != 0 track_usage_fs_op = TrackUsageOp.new(user_id: self.domain.owner._id, parent_user_id: self.domain.owner.parent_user_id, app_name: self.name, gear_id: gear_id, event: UsageRecord::EVENTS[:end], usage_type: UsageRecord::USAGE_TYPES[:addtl_fs_gb], - additional_filesystem_gb: additional_filesystem_gb, + additional_filesystem_gb: additional_filesystem_gb, prereq: [delete_gear_op._id.to_s]) pending_ops.push(track_usage_fs_op) end @@ -1758,7 +1758,7 @@ def calculate_gear_destroy_ops(ginst_id, gear_ids, additional_filesystem_gb) gear_ids.each do |gear_id| pending_ops.push(TrackUsageOp.new(user_id: self.domain.owner._id, parent_user_id: self.domain.owner.parent_user_id, app_name: self.name, gear_id: gear_id, event: UsageRecord::EVENTS[:end], cart_name: comp_spec["cart"], - usage_type: UsageRecord::USAGE_TYPES[:premium_cart], + usage_type: UsageRecord::USAGE_TYPES[:premium_cart], prereq: [delete_gear_op._id.to_s])) end if cartridge.is_premium? end @@ -1912,7 +1912,7 @@ def calculate_add_component_ops(comp_specs, group_instance_id, deploy_gear_id, g if self.scalable expose_port_prereq = [] if post_configure_op - expose_port_prereq << post_configure_op._id.to_s + expose_port_prereq << post_configure_op._id.to_s else expose_port_prereq << add_component_op._id.to_s end @@ -1967,8 +1967,8 @@ def calculate_ops(changes, moves=[], connections=nil, group_overrides=nil, init_ start_order, stop_order = calculate_component_orders unless group_overrides.nil? - set_group_override_op = SetGroupOverridesOp.new(group_overrides: group_overrides, - saved_group_overrides: self.group_overrides, + set_group_override_op = SetGroupOverridesOp.new(group_overrides: group_overrides, + saved_group_overrides: self.group_overrides, pre_save: (not self.persisted?)) pending_ops.push set_group_override_op end @@ -2057,7 +2057,7 @@ def calculate_ops(changes, moves=[], connections=nil, group_overrides=nil, init_ changed_additional_filesystem_gb = change[:to_scale][:additional_filesystem_gb] fs_prereq = [] fs_prereq = [pending_ops.last._id.to_s] if pending_ops.present? - end_usage_op_ids = [] + end_usage_op_ids = [] if change[:from_scale][:additional_filesystem_gb] != 0 group_instance.gears.each do |gear| track_usage_old_fs_op = TrackUsageOp.new(user_id: self.domain.owner._id, parent_user_id: self.domain.owner.parent_user_id, @@ -2160,7 +2160,7 @@ def calculate_ops(changes, moves=[], connections=nil, group_overrides=nil, init_ begin_usage_ops.each {|op| op.prereq += begin_usage_prereq } pending_ops.push *begin_usage_ops end - + [pending_ops, add_gears, remove_gears] end From 77704d4512065e88a2745f7b454aededc3b10849 Mon Sep 17 00:00:00 2001 From: Luke Meyer Date: Thu, 26 Dec 2013 15:09:01 -0500 Subject: [PATCH 2/3] conf to allow alias under cloud domain - bug 1040257 https://bugzilla.redhat.com/show_bug.cgi?id=1040257 --- broker/conf/broker.conf | 5 ++++- broker/config/environments/development.rb | 1 + broker/config/environments/production.rb | 1 + broker/config/environments/test.rb | 1 + controller/app/models/application.rb | 23 +++++++++++++---------- 5 files changed, 20 insertions(+), 11 deletions(-) diff --git a/broker/conf/broker.conf b/broker/conf/broker.conf index 655b47378dd..a77a07b38be 100644 --- a/broker/conf/broker.conf +++ b/broker/conf/broker.conf @@ -126,4 +126,7 @@ HA_DNS_SUFFIX="" #Whether to allow obsolete cartridges to be instantiated for a new application or added to an existing application ALLOW_OBSOLETE_CARTRIDGES="false" - +# Whether to allow users to create aliases that are under the cloud domain, +# potentially conflicting with app names. Note that this will not create any +# entry for the alias in DNS; that would be an external step. +ALLOW_ALIAS_IN_DOMAIN="false" diff --git a/broker/config/environments/development.rb b/broker/config/environments/development.rb index 086720f334b..578b9533155 100644 --- a/broker/config/environments/development.rb +++ b/broker/config/environments/development.rb @@ -78,6 +78,7 @@ config.openshift = { :domain_suffix => conf.get("CLOUD_DOMAIN", "example.com"), + :allow_alias_in_domain => conf.get_bool("ALLOW_ALIAS_IN_DOMAIN", "false"), :default_max_domains => (conf.get("DEFAULT_MAX_DOMAINS", "10")).to_i, :default_max_gears => (conf.get("DEFAULT_MAX_GEARS", "100")).to_i, :default_gear_size => conf.get("DEFAULT_GEAR_SIZE", "small"), diff --git a/broker/config/environments/production.rb b/broker/config/environments/production.rb index e934dc9a115..d2ec6c95250 100644 --- a/broker/config/environments/production.rb +++ b/broker/config/environments/production.rb @@ -67,6 +67,7 @@ config.openshift = { :domain_suffix => conf.get("CLOUD_DOMAIN", "example.com"), + :allow_alias_in_domain => conf.get_bool("ALLOW_ALIAS_IN_DOMAIN", "false"), :default_max_domains => (conf.get("DEFAULT_MAX_DOMAINS", "10")).to_i, :default_max_gears => (conf.get("DEFAULT_MAX_GEARS", "100")).to_i, :default_gear_size => conf.get("DEFAULT_GEAR_SIZE", "small"), diff --git a/broker/config/environments/test.rb b/broker/config/environments/test.rb index 46b60592f9e..113a9d410a6 100644 --- a/broker/config/environments/test.rb +++ b/broker/config/environments/test.rb @@ -77,6 +77,7 @@ config.openshift = { :domain_suffix => conf.get("CLOUD_DOMAIN", "example.com"), + :allow_alias_in_domain => conf.get_bool("ALLOW_ALIAS_IN_DOMAIN", "false"), :default_max_domains => (conf.get("DEFAULT_MAX_DOMAINS", "10")).to_i, :default_max_gears => (conf.get("DEFAULT_MAX_GEARS", "100")).to_i, :default_gear_size => conf.get("DEFAULT_GEAR_SIZE", "small"), diff --git a/controller/app/models/application.rb b/controller/app/models/application.rb index df07013598e..ad3f9b8b855 100644 --- a/controller/app/models/application.rb +++ b/controller/app/models/application.rb @@ -1098,16 +1098,8 @@ def add_alias(fqdn, ssl_certificate=nil, private_key=nil, pass_phrase="") # Alias to be an IP address or a host in the service domain. # Since DNS is case insensitive, all names are downcased for # indexing/compares. - server_alias = fqdn.downcase if fqdn - if (server_alias.nil?) or - (server_alias =~ /#{Rails.configuration.openshift[:domain_suffix]}$/) or - (server_alias.length > 255 ) or - (server_alias.length == 0 ) or - (server_alias =~ /^\d+\.\d+\.\d+\.\d+$/) or - (server_alias =~ /\A[\S]+(\.(json|xml|yml|yaml|html|xhtml))\z/) or - (not server_alias.match(/\A[a-z0-9]+([\.]?[\-a-z0-9]+)+\z/)) - raise OpenShift::UserException.new("The specified alias is not allowed: '#{server_alias}'", 105, "id") - end + server_alias = validate_alias(fqdn) or + raise OpenShift::UserException.new("The specified alias is not allowed: '#{fqdn}'", 105, "id") validate_certificate(ssl_certificate, private_key, pass_phrase) Application.run_in_application_lock(self) do @@ -1124,6 +1116,17 @@ def add_alias(fqdn, ssl_certificate=nil, private_key=nil, pass_phrase="") end end + def validate_alias(fqdn) + return false if fqdn.nil? || fqdn.length > 255 || fqdn.length == 0 + fqdn.downcase! + return false if fqdn =~ /^\d+\.\d+\.\d+\.\d+$/ + return false if fqdn =~ /\A[\S]+(\.(json|xml|yml|yaml|html|xhtml))\z/ + return false if not fqdn =~ /\A[a-z0-9]+([\.]?[\-a-z0-9]+)+\z/ + return false if fqdn.end_with?(Rails.configuration.openshift[:domain_suffix]) && + ! Rails.configuration.openshift[:allow_alias_in_domain] + return fqdn + end + # Removes a DNS alias for the application. # # == Parameters: From 8abd10837a4b9124a0d90d3d2e83a5859b716fa1 Mon Sep 17 00:00:00 2001 From: Luke Meyer Date: Mon, 20 Jan 2014 15:41:52 -0500 Subject: [PATCH 3/3] always prevent alias conflicts with app names --- broker/conf/broker.conf | 7 ++++--- controller/app/models/application.rb | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/broker/conf/broker.conf b/broker/conf/broker.conf index a77a07b38be..32cf8542396 100644 --- a/broker/conf/broker.conf +++ b/broker/conf/broker.conf @@ -126,7 +126,8 @@ HA_DNS_SUFFIX="" #Whether to allow obsolete cartridges to be instantiated for a new application or added to an existing application ALLOW_OBSOLETE_CARTRIDGES="false" -# Whether to allow users to create aliases that are under the cloud domain, -# potentially conflicting with app names. Note that this will not create any -# entry for the alias in DNS; that would be an external step. + +# Whether to allow users to create aliases that are under the cloud domain. Note: +# Aliases of the form word-word. are rejected to prevent conflicts with app names. +# Also this still will not create any DNS entry for the alias; that is an external step. ALLOW_ALIAS_IN_DOMAIN="false" diff --git a/controller/app/models/application.rb b/controller/app/models/application.rb index ad3f9b8b855..d921cf866de 100644 --- a/controller/app/models/application.rb +++ b/controller/app/models/application.rb @@ -1122,8 +1122,11 @@ def validate_alias(fqdn) return false if fqdn =~ /^\d+\.\d+\.\d+\.\d+$/ return false if fqdn =~ /\A[\S]+(\.(json|xml|yml|yaml|html|xhtml))\z/ return false if not fqdn =~ /\A[a-z0-9]+([\.]?[\-a-z0-9]+)+\z/ - return false if fqdn.end_with?(Rails.configuration.openshift[:domain_suffix]) && - ! Rails.configuration.openshift[:allow_alias_in_domain] + if fqdn.end_with?(cloud_domain = Rails.configuration.openshift[:domain_suffix]) + return false if ! Rails.configuration.openshift[:allow_alias_in_domain] + # still exclude those that could conflict with app names. + return false if fqdn.chomp(cloud_domain) =~ /\A\w+-\w+\.\z/ + end return fqdn end