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

Add compliances subcollection #742

Merged

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Feb 19, 2020

Per the convo in the main gitter room last night about api support of compliances February 18, 2020 4:51 PM I thought I may as well go ahead and open this.

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

@d-m-u
Copy link
Contributor Author

d-m-u commented Feb 19, 2020

@abellotti any chance I could get you to take a real quick look, please? thanks.

@miq-bot miq-bot added the wip label Feb 19, 2020
@d-m-u d-m-u force-pushed the adding_subcollection_for_compliance_details branch 2 times, most recently from b9ff9c2 to 16db5df Compare February 19, 2020 13:10
@chessbyte
Copy link
Member

@Real-Omar-Afifi want to try running with this PR to see if it gets you the info you want?

@Fryguy
Copy link
Member

Fryguy commented Feb 19, 2020

I think @Real-Omar-Afifi also need compliance_details, but this can be added as a start and that as a follow up.

@@ -1,6 +1,7 @@
module Api
class VmsController < BaseController
include Subcollections::Disks
include Subcollections::Compliances
Copy link
Member

@Fryguy Fryguy Feb 19, 2020

Choose a reason for hiding this comment

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

Minor, but can you make alphabetical? nvm...nothing else is alphabetical here for some reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i was gonna do a follow-up for it

:get:
- :name: read
:identifier:
- vm_show
Copy link
Member

Choose a reason for hiding this comment

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

compliances are technically on a bunch of different resources. @abellotti Is there a way to do an "OR" of identifiers? Not sure what the right identifiers are, but I usually go with whatever the UI does... @himdel ?

Copy link
Member

Choose a reason for hiding this comment

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

we can specify multiple identifiers (essentially the OR) via :identifiers instead of :identifier

Copy link
Member

@Fryguy Fryguy Feb 19, 2020

Choose a reason for hiding this comment

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

Note, I'm more thinking of if this as a subcollection under other things than just vms.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's a bunch of *_check_compliance features that the UI uses for the "Policy / Check Compliance of the last known configuration" toolbar button.

So, I guess instance_check_compliance, image_check_compliance, vm_check_compliance and miq_template_check_compliance here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I'll get to miq_template_check_compliance in a follow-up

@d-m-u d-m-u force-pushed the adding_subcollection_for_compliance_details branch 2 times, most recently from 594c88e to f82ed6b Compare February 19, 2020 17:09
@d-m-u d-m-u changed the title [WIP] Add compliances subcollection Add compliances subcollection Feb 19, 2020
@abellotti
Copy link
Member

This is great @d-m-u, thanks for adding the tests that exercises the attributes=compliance_details. 👍

@miq-bot miq-bot removed the wip label Feb 19, 2020
@abellotti
Copy link
Member

@d-m-u could you take care of the rubocop warnings ? Thanks.

@d-m-u d-m-u force-pushed the adding_subcollection_for_compliance_details branch 2 times, most recently from fe35a59 to 2f3e190 Compare February 20, 2020 20:02
@d-m-u d-m-u force-pushed the adding_subcollection_for_compliance_details branch from 2f3e190 to 23f0ceb Compare February 20, 2020 20:54
@abellotti
Copy link
Member

@d-m-u try the following:

Move the subcollection action under the vms directly:

  :compliances_subcollection_actions:
    :get:
    - :name: get
      :identifier:
      - vm_show
      - vm_check_compliance

You can see such examples in the :vms: collection.

With this no need to class identify since it's per collection, and the :identifier: handles lists.

Thanks,
Alberto

@d-m-u d-m-u force-pushed the adding_subcollection_for_compliance_details branch from 23f0ceb to a62b5f3 Compare February 20, 2020 21:46
@miq-bot
Copy link
Member

miq-bot commented Feb 20, 2020

Checked commit d-m-u@a62b5f3 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.20.0, and yamllint
3 files checked, 0 offenses detected
Everything looks fine. 🍰

:description: Compliances
:options:
- :subcollection
:verbs: *g
Copy link
Member

Choose a reason for hiding this comment

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

what does the value *g mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the verb set that can be used on the subcollection, in this case just the get

Copy link
Member

@Fryguy Fryguy Feb 21, 2020

Choose a reason for hiding this comment

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

higher-level that is like a "pointer" in YAML, and it means to insert the object referenced with the name "&g". g is defined here:

:g: &g
- :get

@gtanzillo gtanzillo self-assigned this Feb 24, 2020
@gtanzillo gtanzillo added this to the Sprint 131 Ending Mar 2, 2020 milestone Feb 24, 2020
@gtanzillo gtanzillo merged commit 309e538 into ManageIQ:master Feb 24, 2020
@d-m-u d-m-u deleted the adding_subcollection_for_compliance_details branch February 24, 2020 14:44
simaishi pushed a commit that referenced this pull request Feb 24, 2020
@simaishi
Copy link
Contributor

Ivanchuk backport details:

$ git log -1
commit b5cecaef6fcd7d755fb76607b52f6004edfd743a
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Mon Feb 24 09:22:00 2020 -0500

    Merge pull request #742 from d-m-u/adding_subcollection_for_compliance_details

    Add compliances subcollection

    (cherry picked from commit 309e5380171fd3bc8c0b3739c2d04adf6f7e46c0)

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

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.

8 participants