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

Skip hostname validation for virtualization manager #245

Merged
merged 1 commit into from
Apr 9, 2018

Conversation

masayag
Copy link
Contributor

@masayag masayag commented Apr 9, 2018

When adding kubevirt provider with container provider at the same
request, there is no 'default' endpoint if the kubevirt provider is
added first. Therefore no need to check for hostname validation at that
point of time.

When adding kubevirt provider with container provider at the same
request, there is no 'default' endpoint if the kubevirt provider is
added first. Therefore no need to check for hostname validation at that
point of time.
@masayag
Copy link
Contributor Author

masayag commented Apr 9, 2018

Verified by:


cat <<EOF | curl -X POST -H "Content-Type: application/json" -u "admin:smartvm" --data @- "localhost:3000/api/providers"
{
"name": "kubevirt_prov 		",
"zone_id": 1,
  "type": "ManageIQ::Providers::Openshift::ContainerManager",
"connection_configurations": [
     {
       "endpoint": {
         "role": "kubevirt",
         "hostname": "master",
         "port": 8443,
         "security_protocol": "ssl-without-validation"
       },
       "authentication": {
         "role": "kubevirt",
         "auth_key": "token"
       }
     },
     {
       "endpoint": {
         "role": "default",
         "hostname": "master",
         "port": 8443,
         "security_protocol": "ssl-without-validation"
       },
       "authentication": {
         "role": "bearer",
         "auth_key": "token"
       }
     }
   ]
}
EOF

Which returned:

{"results":[{"href":"http://localhost:3000/api/providers/131","id":"131","name":"kubevirt_prov","created_on":"2018-04-09T05:10:45Z","updated_on":"2018-04-09T05:10:45Z","guid":"04edaafd-a932-4610-b3f8-cd9752ccedd1","zone_id":"1","type":"ManageIQ::Providers::Openshift::ContainerManager","api_version":"v1","uid_ems":null,"host_default_vnc_port_start":null,"host_default_vnc_port_end":null,"provider_region":null,"last_refresh_error":null,"last_refresh_date":null,"provider_id":null,"realm":null,"tenant_id":"1","project":null,"parent_ems_id":null,"subscription":null,"last_metrics_error":null,"last_metrics_update_date":null,"last_metrics_success_date":null,"tenant_mapping_enabled":null,"enabled":true,"options":null}]}

While the same script fails without this PR with:
{"error":{"kind":"bad_request","message":"Could not create the new provider - undefined method `ipaddress?' for nil:NilClass","klass":"Api::BadRequestError"}}

@masayag
Copy link
Contributor Author

masayag commented Apr 9, 2018

@moolitayer @zeari please review.

@miq-bot
Copy link
Member

miq-bot commented Apr 9, 2018

Checked commit masayag@d71d556 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@moolitayer
Copy link

This is identical to what @zeari did in #220. Looks good

Copy link

@moolitayer moolitayer left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@moolitayer moolitayer merged commit ee3de02 into ManageIQ:master Apr 9, 2018
@agrare agrare added this to the Sprint 83 Ending Apr 9, 2018 milestone Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants