Updating validation when creating or editing a provider's endpoint.

[yaacov]

Description
A regression was created when introducing multiple provider endpoints to the providers API.
The function for updating authentications did not re-authenticate when changing
an endpoint or an authentication. This commit updates the authentication in this cases.

Issue
#7256

[yaacov]

@miq-bot add_label providers/containers, api

@simon3z @dkorn @abellotti please review

[simon3z]

Style: @yaacov it is possible to have just a one-liner here. Anyway I am not against what you have here ATM.

[yaacov]

provider.authentication_check_types_queue(provider.default_authentication, :save => true) unless data[CONNECTION_ATTR].blank?

@simon3z
this line is ok, but rubocop say it's too long :-( any idea ?

[yaacov]

@simon3z 👍
chenged it to:

endpoints = data[CONNECTION_ATTR]
provider.authentication_check_types_queue(provider.default_authentication, :save => true) unless endpoints.blank?

[simon3z]

LGTM 👍

[yaacov]

@abellotti @imtayadeway please review

[abellotti]

Can we add a test for this ?
Thanks.

[yaacov]

@simon3z @abellotti 👍 👍 about the missing tests

While writing the tests I found the real problem.
It was not in the API, but in the ext_management_system model. The model schedule a new authentication refresh when credentials change, but in our case it did it for :default instead of :bearer.

@kbrock please review

[yaacov]

it's not an API thing now is a core thing :-)

@miq-bot add_label core

@miq-bot remove_label api

cc @abellotti @kbrock

[simon3z]

👍 cc @chessbyte ready for merge

[abellotti]

I'm 👍 with this.

[imtayadeway]

The fix looks good.....I'm just wondering if a) The problem was in the model, and b) The tests aren't concerned with the response from the API, that a model spec would be more appropriate?

[kbrock]

To follow up on @imtayadeway there are [examples] in the tests for testing that method.

Also, if you are testing a particular method, please use a describe at the top level with your method under test. The example I provided you does not do that, so you'd need to add a block for testing connection_configuration_by_role

e.g.:

describe VmOrTemplate do
  describe "#connection_configuration_by_role" do
    # context if you need
    it "works with no parameters" do
      # ...
    end
  end
end

[yaacov]

@kbrock @implowry Thanks 👍

1. Added a spec to test how connection_configuration_by_role behaves when role is missing.
2. Did not remove the tests on the api because the bug/issue was reported on the API [1].

[1] To make this bug happen you need to call the function with missing authentication role. Currently this is the case only in the api's edit action (authentication objects do not have role fields), so the bug of wrongly guessing what is the authentication authtype based on the endpoint role is linked to the api.

@kbrock I used context, because all the tests around do so, I do not know if to describe here ?

[miq-bot]

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

[imtayadeway]

Did not remove the tests on the api because the bug/issue was reported on the API [1].

@yaacov that's fine - I just generally prefer to test only things in the response in request specs, and leave the nitty-gritty things up to lower level tests if their behavior never bubbles up to the user in the response

[yaacov]

@kbrock please re-review

[kbrock]

👍 LGTM - Mr. @abellotti you have the floor

[abellotti]

Thanks @kbrock