Add support for custom endpoint #274
Conversation
| expect(clientid).to eq("klmnopqrst") | ||
| expect(clientkey).to eq("1234567890") | ||
| expect(azure_tenant_id).to eq("abcdefghij") | ||
| expect(subscription).to eq("fghij67890") |
| @e.subscription = "fghij67890" | ||
| @e.endpoint = 'http://www.foo.bar' | ||
| @e.connect | ||
| end |
There was a problem hiding this comment.
@djberg96 - how about a test for a valid endpoint with a path?
There was a problem hiding this comment.
Ok, will add that.
djberg96
changed the title
|
@bronaghs Ok, updated the specs, and even added a few more. |
djberg96
changed the title
djberg96
changed the title
| @@ -25,6 +26,18 @@ def raw_connect(client_id, client_key, azure_tenant_id, subscription, proxy_uri | |||
| $azure_log.warn("No region selected. Validating credentials against public environment.") | |||
| end | |||
|
|
|||
| endpoint_url = endpoint.respond_to?(:url) ? endpoint.url : endpoint.to_s | |||
There was a problem hiding this comment.
endpoint.try(:url) || endpoint.to_s - maybe a little simpler
There was a problem hiding this comment.
True, though I'm hoping ManageIQ/manageiq#17678 will get merged so I can just call to_s.
There was a problem hiding this comment.
Ok, but if ManageIQ/manageiq#17678 doesn't end up getting merged, can you simplify this line?
| environment = ::Azure::Armrest::Environment.discover(:url => endpoint_url, :proxy => proxy_uri) | ||
| rescue SocketError | ||
| raise MiqException::MiqInvalidCredentialsError, _("Invalid endpoint") | ||
| end |
There was a problem hiding this comment.
It basically means the endpoint is invalid, which I'm counting as "credentials" for our purposes. I actually couldn't find a better error, but I'm open to suggestions.
There was a problem hiding this comment.
Maybe this should be switched to an MiqUnreachableError.
Added endpoint and endpoint= methods to CloudManager. Updated connect and raw_connect which now assumes an Endpoint object, and verifies properly. Alter environment error check, add a spec. Added endpoint specs. Added an endpoint with path spec, removed some unnecessary checks. Added endpoint specs. Re-raise a SocketError as an MiqUnreachableError.
|
Checked commit https://github.com/djberg96/manageiq-providers-azure/commit/52c102b79822ad773826e5fc291202ddc219072c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
In order to support the Azure Stack environment we need to allow the users to set a custom endpoint since the default environments will not work. With the help of automatic discovery baked into the azure-armrest gem, we can support this fairly easily. If no endpoint is set then it simply defaults to finding the environment based on region as before.
Partially addresses https://bugzilla.redhat.com/show_bug.cgi?id=1594381
We will still need a modification on the UI side as well.
WIP for now until I add some specs.