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

Jump to bottom

Add subcollection options support for CORS preflight requests #495

Merged
merged 3 commits into from
Oct 18, 2018
Merged

Add subcollection options support for CORS preflight requests #495

merged 3 commits into from
Oct 18, 2018

Conversation

jvlcek
Copy link
Member

@jvlcek jvlcek commented Oct 16, 2018

https://bugzilla.redhat.com/show_bug.cgi?id=1599259

This PR adds support for the options verb on subcollections. This is needed to support
the prefilghted requests done by CORS.

To test this ensure the options verb responds successfully for subcollections
e.g.:

curl -X OPTIONS -k --user admin:<password> https://<API Server>/api/tenants/<id>/quotas

@jvlcek
Copy link
Member Author

jvlcek commented Oct 16, 2018

@miq-bot add_label wip

@jvlcek
Copy link
Member Author

jvlcek commented Oct 16, 2018

@miq-bot add_label bug

@jvlcek
Copy link
Member Author

jvlcek commented Oct 16, 2018

@miq-bot assign @abellotti

@abellotti
Copy link
Member

looks good @jvlcek 👍 just the additional test when you get a chance.

@jvlcek
Copy link
Member Author

jvlcek commented Oct 18, 2018

@miq-bot remove_label wip

@jvlcek jvlcek changed the title [WIP] Add subcollection options support for CORS prefilghted requests Add subcollection options support for CORS prefilghted requests Oct 18, 2018
@miq-bot miq-bot removed the wip label Oct 18, 2018
@jvlcek
Copy link
Member Author

jvlcek commented Oct 18, 2018

@abellotti and @gtanzillo I've made the requested render change and added a spec test.
This should be good to go now. Please take a look.

Thank you for the input.

JoeV

@abellotti abellotti added enhancement and removed bug labels Oct 18, 2018
abellotti
abellotti reviewed Oct 18, 2018
View reviewed changes
spec/requests/tenant_quotas_spec.rb Outdated
@@ -131,6 +131,12 @@
expect(response).to have_http_status(:no_content)
end

it "supports OPTIONS requests on a subcollection without authorization" do
options "/api/tenants/#{tenant.id}/quotas"
Copy link
Member

Choose a reason for hiding this comment

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

use the rails helper for building the url, api_tenant_quotas_url

Copy link
Member Author

Choose a reason for hiding this comment

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

@abellotti OK will do. Thank you!

abellotti
abellotti reviewed Oct 18, 2018
View reviewed changes
spec/requests/tenant_quotas_spec.rb Outdated
@@ -131,6 +131,12 @@
expect(response).to have_http_status(:no_content)
end

it "supports OPTIONS requests on a subcollection without authorization" do
options api_tenant_quotas_url(:c_id => tenant.id)
Copy link
Member

Choose a reason for hiding this comment

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

I thought this was supposed to be api_tenant_quotas_url(nil, tenant)

look at other uses, like api_provider_flavors_url(nil, ems).

@miq-bot
Copy link
Member

miq-bot commented Oct 18, 2018

Checked commits jvlcek/manageiq-api@5edf913~...0e1240b with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🏆

@abellotti
Copy link
Member

Thanks @jvlcek for this API enhancement. LGTM!! 👍

@abellotti abellotti merged commit 3502e51 into ManageIQ:master Oct 18, 2018
@abellotti abellotti changed the title Add subcollection options support for CORS prefilghted requests Add subcollection options support for CORS preflight requests Oct 18, 2018
simaishi pushed a commit that referenced this pull request Oct 18, 2018
@abellotti @simaishi
Merge pull request #495 from jvlcek/bz_1599259_CORS
9b4e420 
Add subcollection options support for CORS prefilghted requests

(cherry picked from commit 3502e51)

https://bugzilla.redhat.com/show_bug.cgi?id=1599259
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 9b4e42018e90d95bc0d46cfe970859ee82386146
Author: Alberto Bellotti <abellotti@users.noreply.github.com>
Date:   Thu Oct 18 16:14:04 2018 -0400

    Merge pull request #495 from jvlcek/bz_1599259_CORS
    
    Add subcollection options support for CORS prefilghted requests
    
    (cherry picked from commit 3502e51181ce92c28866a4626fdfadf0d31bd591)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1599259

@abellotti abellotti added this to the Sprint 97 Ending Oct 22, 2018 milestone Oct 22, 2018
simaishi pushed a commit that referenced this pull request Nov 5, 2018
@abellotti @simaishi
Merge pull request #495 from jvlcek/bz_1599259_CORS
9cc1ee9 
Add subcollection options support for CORS prefilghted requests

(cherry picked from commit 3502e51)

https://bugzilla.redhat.com/show_bug.cgi?id=1646606
@simaishi
Copy link
Contributor

simaishi commented Nov 5, 2018

Gaprindashvili backport details:

$ git log -1
commit 9cc1ee9d5a8e6ad34c6e8846228f78cdd181a57c
Author: Alberto Bellotti <abellotti@users.noreply.github.com>
Date:   Thu Oct 18 16:14:04 2018 -0400

    Merge pull request #495 from jvlcek/bz_1599259_CORS
    
    Add subcollection options support for CORS prefilghted requests
    
    (cherry picked from commit 3502e51181ce92c28866a4626fdfadf0d31bd591)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1646606

@jvlcek jvlcek deleted the bz_1599259_CORS branch June 3, 2019 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abellotti abellotti abellotti approved these changes

Assignees

@abellotti abellotti

Labels
blocker enhancement gaprindashvili/backported hammer/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jvlcek @abellotti @miq-bot @simaishi