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 tags to resource group #238

Merged
merged 6 commits into from
Apr 17, 2020
Merged

Add tags to resource group #238

merged 6 commits into from
Apr 17, 2020

Conversation

rmoles
Copy link
Contributor

@rmoles rmoles commented Apr 17, 2020

Description

  • Adds tags and ids as methods for azurerm_resource_group
  • Change inspec reporter to cli
  • Add pry-byebug to Gemfile
  • Add documentation for tags and ids
  • fix terraform log profile issue

Please describe what this change achieves. Ensure you have read the Contributing to InSpec document before submitting.

Issues Resolved

Fixes #236

Check List

Ross Moles added 6 commits April 17, 2020 12:32
Signed-off-by: Ross Moles <rmoles@chef.io>
Signed-off-by: Ross Moles <rmoles@chef.io>
Signed-off-by: Ross Moles <rmoles@chef.io>
Signed-off-by: Ross Moles <rmoles@chef.io>
Signed-off-by: Ross Moles <rmoles@chef.io>
Signed-off-by: Ross Moles <rmoles@chef.io>
@rmoles rmoles requested a review from a team April 17, 2020 12:55
Copy link

@skpaterson skpaterson left a comment

Choose a reason for hiding this comment

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

Thanks @rmoles !

@skpaterson skpaterson added the Version: Bump Minor Used by github.minor_bump_labels to bump the Minor version number. label Apr 17, 2020
@rmoles rmoles merged commit 2b762c7 into master Apr 17, 2020
@rmoles rmoles deleted the add_tags_to_resource_group branch April 17, 2020 13:06

### names

The names property provides a list of all the Resource Group ids.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it supposed to be: The ids property ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch I'll add a follow-up PR

The names property provides a list of all the Resource Group names.

its('names') { should include 'MyResourceGroup' }

The names property provides a list of all the Resource Group tags.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it supposed to be: The tags property ... ?
And from the error I get using v.1.13.0:


  ×  main: Resource Groups with name == "azrgr-app0-01" (1 failed)
     ✔  Resource Groups with name == "azrgr-app0-01" is expected to exist
     ×  Resource Groups with name == "azrgr-app0-01" tags is expected to include "{ApplicationCode=\"app0\"}"
     expected [{"ApplicationCode" => "app0", "ApplicationName" => "app0", "Branch" => "PARIS", "Environment" => "d", "Exploitation" => "BOOST", "SecurityLevel" => "Standard"}] to include "{ApplicationCode=\"app0\"}"
     Diff:
     @@ -1,2 +1,7 @@
     -["{ApplicationCode=\"app0\"}"]
     +[{"ApplicationCode"=>"app0",
     +  "ApplicationName"=>"app0",
     +  "Branch"=>"PARIS",
     +  "Environment"=>"d",
     +  "Exploitation"=>"BOOST",
     +  "SecurityLevel"=>"Standard"}]

Profile: Azure Resource Pack (inspec-azure)
Version: 1.13.0

it not a list of all tags, it is a list with a single item containing a map representing all tags.

My test code was:

     its('tags') { should include '{ApplicationCode="app0"}' }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't pick this up when testing. Apologies. I'll re-visit this.

Copy link
Contributor Author

@rmoles rmoles Apr 17, 2020

Choose a reason for hiding this comment

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

@rrey Actually looking at your test I think the issue is the quotes surrounding the hash you are checking for. This should work:

    its('tags') { should include({ApplicationCode="app0"}) }

Copy link
Contributor

@rrey rrey Apr 17, 2020

Choose a reason for hiding this comment

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

@rmoles I have a syntax error with your example, error related to "=>" expected instead of the equal sign.
Even with this fixed I get an error:

 ×  main: Resource Groups with name == "azrgr-app0-01" (1 failed)
     ✔  Resource Groups with name == "azrgr-app0-01" is expected to exist
     ×  Resource Groups with name == "azrgr-app0-01" tags
     uninitialized constant #<Class:#<Inspec::ControlEvalContext:0x00007fe9c3ef03d0>>::ApplicationCode


Profile: Azure Resource Pack (inspec-azure)
Version: 1.13.0
Target:  azure://2a8ae5d7-02ef-4f90-8704-20aa8b8bdf87

test code:

    its('tags') { should include({ApplicationCode=>"app0"}) }

Copy link
Contributor Author

@rmoles rmoles Apr 17, 2020

Choose a reason for hiding this comment

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

Ah need to stop context switching on this. Think it's still a formatting error:

    its('tags') { should include({"ApplicationCode"=>"app0"}) }

I've tested this:

  describe azurerm_resource_groups do
    it           { should exist }
    its('names') { should include(resource_group) }
    its('tags')  { should include({"CreatedBy"=>"rmoles"}) }
  end

output:

  ✔  azurerm_resource_groups: Resource Groups
     ✔  Resource Groups is expected to exist
     ✔  Resource Groups names is expected to include "Inspec-Azure-rmoles"
     ✔  Resource Groups tags is expected to include {"CreatedBy" => "rmoles"}

Also tested with the where included:

  describe azurerm_resource_groups.where(name: 'Inspec-Azure-rmoles') do
    it           { should exist }
    its('tags')  { should include({"CreatedBy"=>"rmoles"}) }
  end

Copy link
Contributor

@rrey rrey Apr 17, 2020

Choose a reason for hiding this comment

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

@rmoles how many tags do you have on the resource ?

Edit: It works if I have only one tag on the resource, but I'm using include on a resource with multiple tags.

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 for pointing that out. This was the reason I wasn't able to replicate the issue.
The issue is down to the way the Inspec FilterTable returns the tags as a Hash within an Array.
There are two possible solutions to this problem.

  1. check for the full hash.
its('tags')  { should include({"CreatedBy"=>"rmoles", "blah"=."blah"....}) }
  1. call the .first on the tags method
    its('tags.first')  { should include("CreatedBy"=>"rmoles") }
    its('tags.first')  { should include("tester"=>"test") }

output:

  ✔  azurerm_resource_groups: Resource Groups with name == "Inspec-Azure-rmoles"
     ✔  Resource Groups with name == "Inspec-Azure-rmoles" is expected to exist
     ✔  Resource Groups with name == "Inspec-Azure-rmoles" tags.first is expected to include {"CreatedBy" => "rmoles"}
     ✔  Resource Groups with name == "Inspec-Azure-rmoles" tags.first is expected to include {"tester" => "test"}

Copy link
Contributor

@rrey rrey Apr 17, 2020

Choose a reason for hiding this comment

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

@rmoles
ok, the full hash is working and is fine for me.
Thanks for your time on this !

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.

3 participants