-
Notifications
You must be signed in to change notification settings - Fork 71
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 a test example in google_service_account.md and fix for #200 #199
Conversation
add example in google_service_account.md
merge from remote upstream
Fixes #200 Signed-off-by: Pradeep Bhadani <pradeep.bhadani@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution @pradeepbhadani ! We can proceed with the documentation update here but the other changes will have to be added to Magic Modules - more details in the review comments.
### Test that a GCP project IAM service account does not have user managed keys | ||
|
||
describe google_service_account(name: 'projects/sample-project/serviceAccounts/sample-account@sample-project.iam.gserviceaccount.com') do | ||
its('has_user_managed_keys?') {should cmp false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will work well as written. One possible enhancement is to use some DSL magic along these lines e.g.
it { should have_user_managed_keys }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
@@ -92,10 +92,12 @@ def parse | |||
@logging_service = @fetched['loggingService'] | |||
@monitoring_service = @fetched['monitoringService'] | |||
@network = @fetched['network'] | |||
@network_self_link = @fetched['networkConfig']['network'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks like a great addition. We have two types of resources in InSpec GCP - handwritten and Magic Modules. This resource happens to be generated via Magic Modules so unless this change is made there, it would be overwritten in future updates. There's relevant discussion in this ticket - #162 but feel free to reach out to @slevenick or myself with any questions and we'll be happy to help!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skpaterson - I will take some time to understand how to use Magic Module to make this change.
Is this something related to what I am trying to achieve - https://github.com/GoogleCloudPlatform/magic-modules/blob/master/third_party/terraform/resources/resource_container_cluster.go.erb#L1287?
For now, I will remove this change from this PR.
@private_cluster_config = GoogleInSpec::Container::Property::RegionalClusterPrivateClusterConfig.new(@fetched['privateClusterConfig'], to_s) | ||
@cluster_ipv4_cidr = @fetched['clusterIpv4Cidr'] | ||
@addons_config = GoogleInSpec::Container::Property::RegionalClusterAddonsConfig.new(@fetched['addonsConfig'], to_s) | ||
@subnetwork = @fetched['subnetwork'] | ||
@subnetwork_self_link = @fetched['networkConfig']['subnetwork'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed this change from PR
Signed-off-by: Pradeep Bhadani <pradeep.bhadani@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @pradeepbhadani
Description
service_account
has anuser_managed
key.Issues Resolved
Fixes #193