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

Added parts for the NSX-T provider #825

Merged
merged 10 commits into from
May 27, 2020
Merged

Added parts for the NSX-T provider #825

merged 10 commits into from
May 27, 2020

Conversation

gubbe505
Copy link
Contributor

@gubbe505 gubbe505 commented May 1, 2020

Added network_services, security_policies and security_policy_rules controllers and added some additional api methods to cloud_networks and providers

@gubbe505
Copy link
Contributor Author

gubbe505 commented May 6, 2020

The build fails probably because of this change:

object.respond_to?(:security_groups) ? object.security_groups : {}

In the previous version the security_groups would be converted to an Array and wouldn't be filtered by RBAC. I have looked at other methods and there is no consistent way of returning if the value is nil.

Please advise.

@agrare
Copy link
Member

agrare commented May 6, 2020

@gtanzillo @bdunne are you able to help with the rbac failure here?

@gtanzillo
Copy link
Member

The build fails probably because of this change:

The subquery collection needs to return an array. So this is correct -
object.respond_to?(:security_groups) ? Array(object.security_groups) : []

All results go through Rbac here

def filter_results(miq_expression, res, options)
if miq_expression.present? && options.key?(:limit) && options.key?(:offset)
subquery_res = Rbac.filtered(res, options.except(:offset, :limit))
[Rbac.filtered(res, options), subquery_res.count]
else
[Rbac.filtered(res, options)]
end
end

Looking at https://github.com/ManageIQ/manageiq/blob/048c86151c95786875f00f233704af01b20aec41/lib/rbac/filterer.rb#L60 security groups should get filtered

@gubbe505
Copy link
Contributor Author

gubbe505 commented May 6, 2020

Sorry, my mistake. Had to look up the problem way we changed it. The reason is that mapping to a Array does not support filter as query parameter.

Nevertheless there no consistent way of returning the xxx_query_resource within the api.

@agrare
Copy link
Member

agrare commented May 21, 2020

Hey @gubbe505 any progress on this?

@miq-bot
Copy link
Member

miq-bot commented May 27, 2020

Checked commits gubbe505/manageiq-api@0e4e9d0~...fdbcb0f with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
12 files checked, 0 offenses detected
Everything looks fine. 🏆

@agrare
Copy link
Member

agrare commented May 27, 2020

Hm I can't replicate the travis error locally, it seems like it can't find one of the tags in the region

@agrare
Copy link
Member

agrare commented May 27, 2020

Spec failure looks unrelated to this change, ManageIQ/manageiq#20197

@agrare agrare closed this May 27, 2020
@agrare agrare reopened this May 27, 2020
@abellotti
Copy link
Member

@gubbe505 we seem to be missing the specs here

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.

6 participants