Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bump fog openstack gem to new version 1.1.1 & enable application credential authentication #272

Merged
merged 6 commits into from
May 16, 2024
Merged

bump fog openstack gem to new version 1.1.1 & enable application credential authentication #272

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
beb8b2d
bump fog openstack gem to new version 1.1.1
fmoehler May 15, 2024
2813d65
enable application credential authentication
fmoehler May 15, 2024
4bb28dd
Update cloud.rb
fmoehler May 15, 2024
9cbc810
Update cloud_spec.rb
fmoehler May 15, 2024
5ed8ad2
Remove a deprecated rubocop test
fearoffish May 16, 2024
f416866
Clean up the credentials validation
fearoffish May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions jobs/openstack_cpi/spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@ properties:
- description: Keystone V3 endpoint
value: http://192.168.0.1:5000/v3
openstack.username:
description: OpenStack user name (required)
description: OpenStack user name (required, if application_credential_id and application_credential_secret are not provided)
openstack.api_key:
description: OpenStack API key (required)
description: OpenStack API key (required, if application_credential_id and application_credential_secret are not provided)
openstack.application_credential_id:
description: OpenStack application credential id (required, if username and api_key are not provided)
openstack.application_credential_secret:
description: OpenStack application credential secret (required, if username and api_key are not provided)
openstack.tenant:
description: OpenStack tenant name (required for Keystone API V2)
openstack.project:
Expand Down
36 changes: 19 additions & 17 deletions jobs/openstack_cpi/templates/cpi.json.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
'properties' => {
'openstack' => {
'auth_url' => p('openstack.auth_url'),
'username' => p('openstack.username'),
'api_key' => p('openstack.api_key'),
'default_key_name' => p('openstack.default_key_name'),
'default_security_groups' => p('openstack.default_security_groups'),
'default_volume_type' => p('openstack.default_volume_type', nil),
Expand All @@ -19,21 +17,25 @@
}

openstack_params = params['cloud']['properties']['openstack']
if_p('openstack.region') { |value| openstack_params['region'] = value }
if_p('openstack.endpoint_type') { |value| openstack_params['endpoint_type'] = value }
if_p('openstack.state_timeout') { |value| openstack_params['state_timeout'] = value }
if_p('openstack.stemcell_public_visibility') { |value| openstack_params['stemcell_public_visibility'] = value }
if_p('openstack.connection_options') { |value| openstack_params['connection_options'] = value }
if_p('openstack.boot_from_volume') { |value| openstack_params['boot_from_volume'] = value }
if_p('openstack.config_drive') { |value| openstack_params['config_drive'] = value }
if_p('openstack.use_dhcp') { |value| openstack_params['use_dhcp'] = value }
if_p('openstack.domain') { |value| openstack_params['domain'] = value }
if_p('openstack.user_domain_name') { |value| openstack_params['user_domain_name'] = value }
if_p('openstack.project_domain_name') { |value| openstack_params['project_domain_name'] = value }
if_p('openstack.project') { |value| openstack_params['project'] = value }
if_p('openstack.project_id') { |value| openstack_params['project_id'] = value }
if_p('openstack.tenant') { |value| openstack_params['tenant'] = value }
if_p('openstack.human_readable_vm_names') { |value| openstack_params['human_readable_vm_names'] = value }
if_p('openstack.application_credential_id') { |value| openstack_params['application_credential_id'] = value }
if_p('openstack.application_credential_secret') { |value| openstack_params['application_credential_secret'] = value }
if_p('openstack.username') { |value| openstack_params['username'] = value }
if_p('openstack.api_key') { |value| openstack_params['api_key'] = value }
if_p('openstack.region') { |value| openstack_params['region'] = value }
if_p('openstack.endpoint_type') { |value| openstack_params['endpoint_type'] = value }
if_p('openstack.state_timeout') { |value| openstack_params['state_timeout'] = value }
if_p('openstack.stemcell_public_visibility') { |value| openstack_params['stemcell_public_visibility'] = value }
if_p('openstack.connection_options') { |value| openstack_params['connection_options'] = value }
if_p('openstack.boot_from_volume') { |value| openstack_params['boot_from_volume'] = value }
if_p('openstack.config_drive') { |value| openstack_params['config_drive'] = value }
if_p('openstack.use_dhcp') { |value| openstack_params['use_dhcp'] = value }
if_p('openstack.domain') { |value| openstack_params['domain'] = value }
if_p('openstack.user_domain_name') { |value| openstack_params['user_domain_name'] = value }
if_p('openstack.project_domain_name') { |value| openstack_params['project_domain_name'] = value }
if_p('openstack.project') { |value| openstack_params['project'] = value }
if_p('openstack.project_id') { |value| openstack_params['project_id'] = value }
if_p('openstack.tenant') { |value| openstack_params['tenant'] = value }
if_p('openstack.human_readable_vm_names') { |value| openstack_params['human_readable_vm_names'] = value }

if_p('openstack.enable_auto_anti_affinity') do
raise "Property 'enable_auto_anti_affinity' is no longer supported. Please remove it from your configuration."
Expand Down
2 changes: 1 addition & 1 deletion src/bosh_openstack_cpi/Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ source 'https://rubygems.org'
gem 'bosh_common'
gem 'bosh_cpi', '~>2.5.0'
gem 'excon', '~>0.71.0'
gem 'fog-openstack', '~>1.1.0'
gem 'fog-openstack', '~>1.1.1'
gem 'httpclient', '=2.8.3'
gem 'membrane', '~>1.1.0'
gem 'netaddr', '~>2.0.4'
Expand Down
4 changes: 2 additions & 2 deletions src/bosh_openstack_cpi/Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ GEM
fog-json (1.2.0)
fog-core
multi_json (~> 1.10)
fog-openstack (1.1.0)
fog-openstack (1.1.1)
fog-core (~> 2.1)
fog-json (>= 1.0)
formatador (1.1.0)
Expand Down Expand Up @@ -116,7 +116,7 @@ DEPENDENCIES
bosh_common
bosh_cpi (~> 2.5.0)
excon (~> 0.71.0)
fog-openstack (~> 1.1.0)
fog-openstack (~> 1.1.1)
httpclient (= 2.8.3)
membrane (~> 1.1.0)
minitar (~> 0.5.4)
Expand Down
18 changes: 18 additions & 0 deletions src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,10 @@ def validate_options
user_domain_name = @options['openstack']['user_domain_name']
project_domain_name = @options['openstack']['project_domain_name']
project_id = @options['openstack']['project_id']
username = @options['openstack']['username']
api_key = @options['openstack']['api_key']
application_credential_id = @options['openstack']['application_credential_id']
application_credential_secret = @options['openstack']['application_credential_secret']
schema = Membrane::SchemaParser.parse do
openstack_options_schema = {
'openstack' => {
Expand Down Expand Up @@ -778,6 +782,20 @@ def validate_options
},
optional('agent') => Hash,
}

unless ((username && api_key) || (application_credential_id && application_credential_secret)) ||
(username && api_key && application_credential_id && application_credential_secret)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we check with (username && api_key && application_credential_id && application_credential_secret) ?
Finally all of the properties must not be nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We wanted to check that not all of these credentials are provided.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then it should be

unless ((username && api_key) || (application_credential_id && application_credential_secret)) &&
          !(username && api_key && application_credential_id && application_credential_secret)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, may be splitting this into two boolean variables make it more readable?
e.g.: using_api_key = username && api_key?

raise ArgumentError, "Invalid OpenStack cloud properties: '#{username} and #{api_key}' or '#{application_credential_id} and #{application_credential_secret}' is required."
end

if username && api_key
openstack_options_schema['openstack']['username'] = String
openstack_options_schema['openstack']['api_key'] = String
elsif application_credential_id && application_credential_secret
openstack_options_schema['openstack']['application_credential_id'] = String
openstack_options_schema['openstack']['application_credential_secret'] = String
end

if Bosh::OpenStackCloud::Openstack.is_v3(auth_url)
if project_id
openstack_options_schema['openstack']['project_id'] = String
Expand Down
2 changes: 2 additions & 0 deletions src/bosh_openstack_cpi/lib/cloud/openstack/openstack.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,8 @@ def openstack_params(options)
openstack_auth_url: auth_url,
openstack_username: options['username'],
openstack_api_key: options['api_key'],
openstack_application_credential_id: options['application_credential_id'],
openstack_application_credential_secret: options['application_credential_secret'],
openstack_tenant: options['tenant'],
openstack_project_name: options['project'],
openstack_project_id: options['project_id'],
Expand Down
51 changes: 51 additions & 0 deletions src/bosh_openstack_cpi/spec/unit/cloud_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,57 @@
end
end

context 'when checking authentication' do
before do
options['openstack']['auth_url'] = 'http://fake-auth-url/v2.0'
options['openstack']['tenant'] = 'fake-tenant'
end

it 'raises ArgumentError if there are no authentication credentials provided' do
options['openstack']['username'] = nil
options['openstack']['api_key'] = nil

expect { subject }.to raise_error(ArgumentError, /Invalid OpenStack cloud properties/)
end

it 'raises ArgumentError if only api_key is provided' do
options['openstack']['api_key'] = nil

expect { subject }.to raise_error(ArgumentError, /Invalid OpenStack cloud properties/)
end

it 'raises ArgumentError if only username is provided' do
options['openstack']['username'] = nil

expect { subject }.to raise_error(ArgumentError, /Invalid OpenStack cloud properties/)
end

it 'does not raise an error if application credential id and secret are provided' do
options['openstack']['api_key'] = nil
options['openstack']['username'] = nil
options['openstack']['application_credential_id'] = 'fake-application-credential-id'
options['openstack']['application_credential_secret'] = 'fake-application-credential-secret'

expect { subject }.to_not raise_error
end

it 'raises ArgumentError if only application credential id is provided' do
options['openstack']['api_key'] = nil
options['openstack']['username'] = nil
options['openstack']['application_credential_id'] = 'fake-application-credential-id'

expect { subject }.to raise_error(ArgumentError, /Invalid OpenStack cloud properties/)
end

it 'raises ArgumentError if only application credential secret is provided' do
options['openstack']['api_key'] = nil
options['openstack']['username'] = nil
options['openstack']['application_credential_secret'] = 'fake-application-credential-secret'

expect { subject }.to raise_error(ArgumentError, /Invalid OpenStack cloud properties/)
end
end

end

context 'when options are empty' do
Expand Down