Skip to content

Commit

Permalink
Revert "Add environment_variables field to V3 Apps"
Browse files Browse the repository at this point in the history
This reverts commit 87c85e3.
  • Loading branch information
luan committed Mar 20, 2015
1 parent af5be06 commit 2bd7d93
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 163 deletions.
1 change: 0 additions & 1 deletion app/actions/app_update.rb
Expand Up @@ -8,7 +8,6 @@ def self.update(app, message)
app.lock!

app.name = message['name'] unless message['name'].nil?
app.environment_variables = message['environment_variables'] unless message['environment_variables'].nil?

if message['desired_droplet_guid']
droplet = DropletModel.find(guid: message['desired_droplet_guid'])
Expand Down
18 changes: 0 additions & 18 deletions app/models/v3/persistence/app_model.rb
Expand Up @@ -9,28 +9,10 @@ class AppModel < Sequel::Model(:apps_v3)
one_to_many :packages, class: 'VCAP::CloudController::PackageModel', key: :app_guid, primary_key: :guid
one_to_many :droplets, class: 'VCAP::CloudController::DropletModel', key: :app_guid, primary_key: :guid

encrypt :environment_variables, salt: :salt, column: :encrypted_environment_variables

def validate
validates_presence :name
validates_unique [:space_guid, :name]
validates_format APP_NAME_REGEX, :name
validate_environment_variables
end

def validate_environment_variables
return unless environment_variables
keys = environment_variables.keys
keys.each do |key|
key = key.to_s
if key =~ /^CF_/i
errors.add(:environment_variables, 'cannot start with CF_')
elsif key =~ /^VCAP_/i
errors.add(:environment_variables, 'cannot start with VCAP_')
elsif key == 'PORT'
errors.add(:environment_variables, 'cannot set PORT')
end
end
end

def self.user_visible(user)
Expand Down
12 changes: 5 additions & 7 deletions app/presenters/v3/app_presenter.rb
Expand Up @@ -25,14 +25,12 @@ def present_json_list(paginated_result, facets={})
private

def app_hash(app)
hash = {
guid: app.guid,
name: app.name,
desired_state: app.desired_state,
_links: build_links(app),
{
guid: app.guid,
name: app.name,
desired_state: app.desired_state,
_links: build_links(app),
}
hash[:environment_variables] = app.environment_variables if app.environment_variables
hash
end

def build_links(app)
Expand Down
4 changes: 2 additions & 2 deletions lib/cloud_controller/encryptor.rb
Expand Up @@ -12,12 +12,12 @@ def generate_salt

def encrypt(input, salt)
return nil unless input
Base64.strict_encode64(run_cipher(make_cipher.encrypt, YAML.dump(input), salt))
Base64.strict_encode64(run_cipher(make_cipher.encrypt, input, salt))
end

def decrypt(encrypted_input, salt)
return nil unless encrypted_input
YAML.load(run_cipher(make_cipher.decrypt, Base64.decode64(encrypted_input), salt))
run_cipher(make_cipher.decrypt, Base64.decode64(encrypted_input), salt)
end

attr_accessor :db_encryption_key
Expand Down
8 changes: 0 additions & 8 deletions spec/api/documentation/v3/apps_api_spec.rb
Expand Up @@ -203,16 +203,9 @@ def space_guid_facets(space_guids)

parameter :name, 'Name of the App'
parameter :desired_droplet_guid, 'GUID of the Droplet to be used for the App'
parameter :environment_variables, 'Environment variables to be used for the App when running'

let(:name) { 'new_name' }
let(:desired_droplet_guid) { droplet.guid }
let(:environment_variables) do
{
'MY_ENV_VAR' => 'foobar',
'FOOBAR' => 'MY_ENV_VAR'
}
end
let(:guid) { app_model.guid }

let(:raw_post) { MultiJson.dump(params, pretty: true) }
Expand All @@ -224,7 +217,6 @@ def space_guid_facets(space_guids)
'name' => name,
'guid' => app_model.guid,
'desired_state' => app_model.desired_state,
'environment_variables' => environment_variables,
'_links' => {
'self' => { 'href' => "/v3/apps/#{app_model.guid}" },
'processes' => { 'href' => "/v3/apps/#{app_model.guid}/processes" },
Expand Down
14 changes: 1 addition & 13 deletions spec/unit/actions/app_update_spec.rb
Expand Up @@ -6,7 +6,7 @@ module VCAP::CloudController
let(:app_update) { AppUpdate }
let(:app_model) { AppModel.make }

describe '.update' do
describe '#update' do
context 'when the desired_droplet does not exist' do
let(:message) { { 'desired_droplet_guid' => 'not_a_guid' } }

Expand Down Expand Up @@ -55,18 +55,6 @@ module VCAP::CloudController
end
end

context 'when updating the environment variables' do
let(:environment_variables) { { 'VARIABLE' => 'VALUE' } }
let(:message) { { 'environment_variables' => environment_variables } }

it 'updates the app name' do
app_update.update(app_model, message)
app_model.reload

expect(app_model.environment_variables).to eq(environment_variables)
end
end

context 'when the app is invalid' do
let(:name) { 'new name' }
let(:message) { { 'name' => name } }
Expand Down
20 changes: 0 additions & 20 deletions spec/unit/controllers/v3/apps_controller_spec.rb
Expand Up @@ -280,7 +280,6 @@ module VCAP::CloudController
_, response = apps_controller.update(app_model.guid)
expect(response).to eq(app_response)
end

context 'when the app is invalid' do
before do
allow(AppUpdate).to receive(:update).and_raise(AppUpdate::InvalidApp.new('ya done goofed'))
Expand Down Expand Up @@ -311,25 +310,6 @@ module VCAP::CloudController
end
end
end

context 'when the user attempts to set a reserved environment variable' do
let(:req_body) do
{
environment_variables: {
CF_GOOFY_GOOF: 'you done goofed!'
}
}.to_json
end

it 'returns the correct error' do
expect {
apps_controller.update(app_model.guid)
}.to raise_error do |error|
expect(error.name).to eq('UnprocessableEntity')
expect(error.message).to match('The request is semantically invalid: environment_variables cannot start with CF_')
end
end
end
end
end

Expand Down
5 changes: 2 additions & 3 deletions spec/unit/handlers/apps_handler_spec.rb
Expand Up @@ -14,7 +14,7 @@ module VCAP::CloudController
AppModel.make(name: app_1.name, space_guid: space_3.guid)
end

it 'filters by access_context' do
it 'filters by faccess_contextets' do
access_context = double(:access_context, roles: double(:roles, admin?: true))
apps_repository = AppsRepository.new

Expand All @@ -23,8 +23,7 @@ module VCAP::CloudController
'space_guids' => [space_1.guid, space_2.guid]
}).all

expect(apps.length).to eq(2)
expect(apps).to include(app_1, app_2)
expect(apps).to eq([app_1, app_2])
end

it 'filters by orgs' do
Expand Down
37 changes: 0 additions & 37 deletions spec/unit/lib/cloud_controller/encryptor_spec.rb
Expand Up @@ -47,43 +47,6 @@ module VCAP::CloudController
end
end
end

describe 'encrypting a hash' do
let(:input) { { a: 'hash' } }
let!(:encrypted_string) { Encryptor.encrypt(input, salt) }

it 'returns an encrypted string' do
expect(encrypted_string).to match(/\w+/)
expect(encrypted_string).not_to include(YAML.dump(input))
end

it 'is deterministic' do
expect(Encryptor.encrypt(input, salt)).to eql(encrypted_string)
end

it 'depends on the salt' do
expect(Encryptor.encrypt(input, Encryptor.generate_salt)).not_to eql(encrypted_string)
end

it 'depends on the db_encryption_key from the CC config file' do
allow(VCAP::CloudController::Encryptor).to receive(:db_encryption_key).and_return('a-totally-different-key')
expect(Encryptor.encrypt(input, salt)).not_to eql(encrypted_string)
end

it 'does not encrypt null values' do
expect(Encryptor.encrypt(nil, salt)).to be_nil
end

describe 'decrypting the hash' do
it 'returns the original hash' do
expect(Encryptor.decrypt(encrypted_string, salt)).to eq(input)
end

it 'returns nil if the encrypted input is nil' do
expect(Encryptor.decrypt(nil, salt)).to be_nil
end
end
end
end

describe Encryptor::FieldEncryptor do
Expand Down
42 changes: 22 additions & 20 deletions spec/unit/lib/db/db_standards_spec.rb
@@ -1,27 +1,29 @@
require 'spec_helper'

describe 'DB Schema' do
DbConfig.connection.tables.each do |table|
it "the table #{table}'s name should not be longer than 60 characters" do
expect(table.length).to be <= 60
end

DbConfig.connection.schema(table).each do |column|
it "the column #{table}.#{column}'s name should not be longer than 60 characters" do
expect(column[0].length).to be <= 60
context 'To support Oracle' do
DbConfig.connection.tables.each do |table|
it "the table #{table}'s name should not be longer than 30 characters" do
expect(table.length).to be <= 30
end
end if DbConfig.connection.supports_schema_parsing?

DbConfig.connection.foreign_key_list(table).each do |fk|
it "the foreign key #{table}.#{fk[:name]}'s name should not be longer than 60 characters" do
expect(fk[:name].length).to be <= 60
end unless fk[:name].nil?
end if DbConfig.connection.supports_foreign_key_parsing?
DbConfig.connection.schema(table).each do |column|
it "the column #{table}.#{column}'s name should not be longer than 30 characters" do
expect(column[0].length).to be <= 30
end
end if DbConfig.connection.supports_schema_parsing?

DbConfig.connection.indexes(table).each do |name, index|
it "the index #{table}.#{name}'s name should not be longer than 60 characters" do
expect(name.length).to be <= 60
end
end if DbConfig.connection.supports_index_parsing?
end if DbConfig.connection.supports_table_listing?
DbConfig.connection.foreign_key_list(table).each do |fk|
it "the foreign key #{table}.#{fk[:name]}'s name should not be longer than 30 characters" do
expect(fk[:name].length).to be <= 30
end unless fk[:name].nil?
end if DbConfig.connection.supports_foreign_key_parsing?

DbConfig.connection.indexes(table).each do |name, index|
it "the index #{table}.#{name}'s name should not be longer than 30 characters" do
expect(name.length).to be <= 30
end
end if DbConfig.connection.supports_index_parsing?
end if DbConfig.connection.supports_table_listing?
end
end
32 changes: 0 additions & 32 deletions spec/unit/models/runtime/v3/persistence/app_model_spec.rb
Expand Up @@ -117,38 +117,6 @@ module VCAP::CloudController
}.to raise_error(Sequel::ValidationFailed, /space_guid and name/)
end
end

describe 'environment_variables' do
it 'does not allow variables that start with CF_' do
expect {
AppModel.make(environment_variables: { CF_POTATO: 'muy bueno' })
}.to raise_error(Sequel::ValidationFailed, /cannot start with CF_/)
end

it 'does not allow variables that start with cf_' do
expect {
AppModel.make(environment_variables: { cf_potato: 'muy bueno' })
}.to raise_error(Sequel::ValidationFailed, /cannot start with CF_/)
end

it 'does not allow variables that start with VCAP_' do
expect {
AppModel.make(environment_variables: { VCAP_BANANA: 'no bueno' })
}.to raise_error(Sequel::ValidationFailed, /cannot start with VCAP_/)
end

it 'does not allow variables that start with vcap_' do
expect {
AppModel.make(environment_variables: { vcap_banana: 'no bueno' })
}.to raise_error(Sequel::ValidationFailed, /cannot start with VCAP_/)
end

it 'does not allow PORT' do
expect {
AppModel.make(environment_variables: { PORT: 'el martes nos ponemos camisetas naranjas' })
}.to raise_error(Sequel::ValidationFailed, /cannot set PORT/)
end
end
end
end
end
3 changes: 1 addition & 2 deletions spec/unit/presenters/v3/app_presenter_spec.rb
Expand Up @@ -5,15 +5,14 @@ module VCAP::CloudController
describe AppPresenter do
describe '#present_json' do
it 'presents the app as json' do
app = AppModel.make(environment_variables: { 'some' => 'stuff' }, desired_state: 'STOPPED')
app = AppModel.make

json_result = AppPresenter.new.present_json(app)
result = MultiJson.load(json_result)

expect(result['guid']).to eq(app.guid)
expect(result['name']).to eq(app.name)
expect(result['desired_state']).to eq(app.desired_state)
expect(result['environment_variables']).to eq(app.environment_variables)
expect(result['_links']).not_to include('desired_droplet')
end

Expand Down

0 comments on commit 2bd7d93

Please sign in to comment.