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.

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

API: Allow security_protocol specified during provider creation #9570

Merged
merged 1 commit into from Jul 8, 2016

Conversation

isimluk
Copy link
Member

@isimluk isimluk commented Jul 1, 2016

Purpose or Intent

Fix minor api regression. And write relevant spec.

Links

@miq-bot add_label api, bug, providers, darga/yes
@miq-bot assign @abellotti

@@ -5,7 +5,7 @@ module Providers
CREDENTIALS_ATTR = "credentials"
AUTH_TYPE_ATTR = "auth_type"
DEFAULT_AUTH_TYPE = "default"
ENDPOINT_ATTRS = %w(hostname ipaddress port)
ENDPOINT_ATTRS = %w(hostname ipaddress port security_protocol).freeze
Copy link
Member

Choose a reason for hiding this comment

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

I see other attributes being delegated to Endpoint in the newer providers (app/models/providers.rb), like verify_ssl, url, pah, etc. I wonder if we shouldn't get this list from the Endpoint model, i.e. Endpoint.columns_hash.keys or something like Endpoint.supported_attributes

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the app/models/provider.rb is not relevant here.

The problem is that Provider and ExtManagementSystem class each delegate different set of attributes to a default endpoint. Further, default endpoint of a Provider is just first default endpoint of any ExtManagementSystems assigned to the Provider.

More importantly, the '/api/providers' manages only ExtManagementSystem records. We may want to add support for Provider records. Do we have a ticket for this in backlog somewhere?

Currently, there are only three sub-classes of the Provider class.

$ git grep '<\s*[:]*Provider'
app/models/manageiq/providers/ansible_tower/provider.rb:class ManageIQ::Providers::AnsibleTower::Provider < ::Provider
app/models/manageiq/providers/foreman/provider.rb:class ManageIQ::Providers::Foreman::Provider < ::Provider
app/models/manageiq/providers/openstack/provider.rb:class ManageIQ::Providers::Openstack::Provider < ::Provider

Copy link
Member

Choose a reason for hiding this comment

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

we support both types with /api/providers, see provider_class="provider" parameter. This capability was added when Foreman was being introduced.

@miq-bot
Copy link
Member

miq-bot commented Jul 7, 2016

Checked commit isimluk@7731386 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
2 files checked, 0 offenses detected
Everything looks good. ⭐

@abellotti
Copy link
Member

👍 we can always extend in future to have end point attrs to be provider specific.

@abellotti abellotti merged commit 0a5456b into ManageIQ:master Jul 8, 2016
@abellotti abellotti added this to the Sprint 43 Ending July 11, 2016 milestone Jul 8, 2016
chessbyte pushed a commit that referenced this pull request Jul 11, 2016
API: Allow security_protocol specified during provider creation
(cherry picked from commit 0a5456b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants