Skip to content

Allow import steps in beta IAM resource tests #7185

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

Merged
SarahFrench merged 5 commits into from
Feb 20, 2023
Merged

Allow import steps in beta IAM resource tests #7185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
46f9d70
Remove ga-only logic from IAM test generation
SarahFrench Sep 20, 2022
04255f9
Add ability to skip import steps in IAM resource tests
SarahFrench Feb 7, 2023
2e04d34
Add `skip_import_test` to beta tests that have broken import steps
SarahFrench Feb 7, 2023
00b59f6
Add `skip_import_test` to Dataproc Metastore IAM tests
SarahFrench Feb 7, 2023
1d04094
Add `skip_import_test` to Cloud Build v2 IAM tests
SarahFrench Feb 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions mmv1/api/resource/iam_policy.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ class IamPolicy < Api::Object
# boolean of if this binding should be generated
attr_reader :exclude_validator

# Boolean of if tests for IAM resources should exclude import test steps
# Used to handle situations where typical generated IAM tests cannot import
# due to the parent resource having an API-generated id
attr_reader :skip_import_test

# Character that separates resource identifier from method call in URL
# For example, PubSub subscription uses {resource}:getIamPolicy
# While Compute subnetwork uses {resource}/getIamPolicy
Expand Down Expand Up @@ -110,6 +115,7 @@ def validate

check :exclude, type: :boolean, default: false
check :exclude_validator, type: :boolean, default: false
check :skip_import_test, type: :boolean, default: false
check :method_name_separator, type: String, default: '/'
check :parent_resource_type, type: String
check :fetch_iam_policy_verb, type: Symbol, default: :GET, allowed: %i[GET POST]
Expand Down
3 changes: 3 additions & 0 deletions mmv1/products/apigateway/terraform.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides
Api: !ruby/object:Overrides::Terraform::ResourceOverride
autogen_async: true
iam_policy: !ruby/object:Api::Resource::IamPolicy
skip_import_test: true
allowed_iam_role: 'roles/apigateway.viewer'
method_name_separator: ':'
parent_resource_attribute: 'api'
Expand Down Expand Up @@ -47,6 +48,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides
specified prefix. If this and api_config_id are unspecified, a random value is chosen for the name.
autogen_async: true
iam_policy: !ruby/object:Api::Resource::IamPolicy
skip_import_test: true
allowed_iam_role: 'roles/apigateway.viewer'
parent_resource_attribute: api_config
base_url: projects/{{project}}/locations/global/apis/{{api}}/configs/{{api_config}}
Expand Down Expand Up @@ -111,6 +113,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides
Gateway: !ruby/object:Overrides::Terraform::ResourceOverride
autogen_async: true
iam_policy: !ruby/object:Api::Resource::IamPolicy
skip_import_test: true
allowed_iam_role: 'roles/apigateway.viewer'
method_name_separator: ':'
parent_resource_attribute: 'gateway'
Expand Down
1 change: 1 addition & 0 deletions mmv1/products/cloudbuildv2/terraform.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides
Connection: !ruby/object:Overrides::Terraform::ResourceOverride
exclude_validator: true
iam_policy: !ruby/object:Api::Resource::IamPolicy
skip_import_test: true
min_version: beta
exclude: false
method_name_separator: ':'
Expand Down
1 change: 1 addition & 0 deletions mmv1/products/compute/terraform.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides
machineImageEncryptionKey.kmsKeyName: !ruby/object:Overrides::Terraform::PropertyOverride
diff_suppress_func: compareCryptoKeyVersions
iam_policy: !ruby/object:Api::Resource::IamPolicy
skip_import_test: true
allowed_iam_role: 'roles/compute.admin'
parent_resource_attribute: 'machine_image'
iam_conditions_request_type: :QUERY_PARAM
Expand Down
2 changes: 2 additions & 0 deletions mmv1/products/datacatalog/api.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ objects:
'Official Documentation': https://cloud.google.com/data-catalog/docs
api: https://cloud.google.com/data-catalog/docs/reference/rest/v1beta1/projects.locations.taxonomies
iam_policy: !ruby/object:Api::Resource::IamPolicy
skip_import_test: true
method_name_separator: ':'
fetch_iam_policy_verb: :POST
parent_resource_attribute: 'taxonomy'
Expand Down Expand Up @@ -572,6 +573,7 @@ objects:
'Official Documentation': https://cloud.google.com/data-catalog/docs
api: https://cloud.google.com/data-catalog/docs/reference/rest/v1beta1/projects.locations.taxonomies.policyTags
iam_policy: !ruby/object:Api::Resource::IamPolicy
skip_import_test: true
method_name_separator: ':'
fetch_iam_policy_verb: :POST
parent_resource_attribute: 'policy_tag'
Expand Down
1 change: 1 addition & 0 deletions mmv1/products/metastore/api.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ objects:
description: |
A managed metastore federation.
iam_policy: !ruby/object:Api::Resource::IamPolicy
skip_import_test: true
parent_resource_attribute: federation_id
exclude: false
method_name_separator: ':'
Expand Down
2 changes: 2 additions & 0 deletions mmv1/products/servicedirectory/api.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ objects:
'Configuring a namespace': 'https://cloud.google.com/service-directory/docs/configuring-service-directory#configuring_a_namespace'
api: 'https://cloud.google.com/service-directory/docs/reference/rest/v1beta1/projects.locations.namespaces'
iam_policy: !ruby/object:Api::Resource::IamPolicy
skip_import_test: true
exclude: false
parent_resource_attribute: 'name'
method_name_separator: ':'
Expand Down Expand Up @@ -88,6 +89,7 @@ objects:
'Configuring a service': 'https://cloud.google.com/service-directory/docs/configuring-service-directory#configuring_a_service'
api: 'https://cloud.google.com/service-directory/docs/reference/rest/v1beta1/projects.locations.namespaces.services'
iam_policy: !ruby/object:Api::Resource::IamPolicy
skip_import_test: true
exclude: false
parent_resource_attribute: 'name'
method_name_separator: ':'
Expand Down
30 changes: 10 additions & 20 deletions mmv1/templates/terraform/examples/base_configs/iam_test_file.go.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
package google

import (
<% if object.min_version.name == "ga" -%>
"fmt"
<% end -%>
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
Expand Down Expand Up @@ -76,7 +74,7 @@ func TestAcc<%= resource_name -%>IamBindingGenerated(t *testing.T) {
{
Config: testAcc<%= resource_name -%>IamBinding_basicGenerated(context),
},
<% if object.min_version.name == "ga" -%>
<% unless object.iam_policy.skip_import_test -%>
{
ResourceName: "<%= resource_ns_iam -%>_binding.foo",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%>"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>),
Expand All @@ -88,7 +86,7 @@ func TestAcc<%= resource_name -%>IamBindingGenerated(t *testing.T) {
// Test Iam Binding update
Config: testAcc<%= resource_name -%>IamBinding_updateGenerated(context),
},
<% if object.min_version.name == "ga" -%>
<% unless object.iam_policy.skip_import_test -%>
{
ResourceName: "<%= resource_ns_iam -%>_binding.foo",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%>"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>),
Expand Down Expand Up @@ -123,7 +121,7 @@ func TestAcc<%= resource_name -%>IamMemberGenerated(t *testing.T) {
// Test Iam Member creation (no update for member, no need to test)
Config: testAcc<%= resource_name -%>IamMember_basicGenerated(context),
},
<% if object.min_version.name == "ga" -%>
<% unless object.iam_policy.skip_import_test -%>
{
ResourceName: "<%= resource_ns_iam -%>_member.foo",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%> user:[email protected]"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>),
Expand Down Expand Up @@ -164,7 +162,7 @@ func TestAcc<%= resource_name -%>IamPolicyGenerated(t *testing.T) {
{
Config: testAcc<%= resource_name -%>IamPolicy_basicGenerated(context),
},
<% if object.min_version.name == "ga" -%>
<% unless object.iam_policy.skip_import_test -%>
{
ResourceName: "<%= resource_ns_iam -%>_policy.foo",
ImportStateId: fmt.Sprintf("<%= import_url -%>"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>),
Expand All @@ -175,7 +173,7 @@ func TestAcc<%= resource_name -%>IamPolicyGenerated(t *testing.T) {
{
Config: testAcc<%= resource_name -%>IamPolicy_emptyBinding(context),
},
<% if object.min_version.name == "ga" -%>
<% unless object.iam_policy.skip_import_test -%>
{
ResourceName: "<%= resource_ns_iam -%>_policy.foo",
ImportStateId: fmt.Sprintf("<%= import_url -%>"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>),
Expand Down Expand Up @@ -210,7 +208,7 @@ func TestAcc<%= resource_name -%>IamBindingGenerated_withCondition(t *testing.T)
{
Config: testAcc<%= resource_name -%>IamBinding_withConditionGenerated(context),
},
<% if object.min_version.name == "ga" -%>
<% unless object.iam_policy.skip_import_test -%>
{
ResourceName: "<%= resource_ns_iam -%>_binding.foo",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%> %s"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>, context["condition_title"]),
Expand Down Expand Up @@ -246,23 +244,19 @@ func TestAcc<%= resource_name -%>IamBindingGenerated_withAndWithoutCondition(t *
{
Config: testAcc<%= resource_name -%>IamBinding_withAndWithoutConditionGenerated(context),
},
<% if object.min_version.name == "ga" -%>
<% unless object.iam_policy.skip_import_test -%>
{
ResourceName: "<%= resource_ns_iam -%>_binding.foo",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%>"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>),
ImportState: true,
ImportStateVerify: true,
},
<% end -%>
<% if object.min_version.name == "ga" -%>
{
ResourceName: "<%= resource_ns_iam -%>_binding.foo2",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%> %s"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>, context["condition_title"]),
ImportState: true,
ImportStateVerify: true,
},
<% end -%>
<% if object.min_version.name == "ga" -%>
{
ResourceName: "<%= resource_ns_iam -%>_binding.foo3",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%> %s"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>, context["condition_title_no_desc"]),
Expand Down Expand Up @@ -296,7 +290,7 @@ func TestAcc<%= resource_name -%>IamMemberGenerated_withCondition(t *testing.T)
{
Config: testAcc<%= resource_name -%>IamMember_withConditionGenerated(context),
},
<% if object.min_version.name == "ga" -%>
<% unless object.iam_policy.skip_import_test -%>
{
ResourceName: "<%= resource_ns_iam -%>_member.foo",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%> user:[email protected] %s"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>, context["condition_title"]),
Expand Down Expand Up @@ -332,23 +326,19 @@ func TestAcc<%= resource_name -%>IamMemberGenerated_withAndWithoutCondition(t *t
{
Config: testAcc<%= resource_name -%>IamMember_withAndWithoutConditionGenerated(context),
},
<% if object.min_version.name == "ga" -%>
<% unless object.iam_policy.skip_import_test -%>
{
ResourceName: "<%= resource_ns_iam -%>_member.foo",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%> user:[email protected]"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>),
ImportState: true,
ImportStateVerify: true,
},
<% end -%>
<% if object.min_version.name == "ga" -%>
{
ResourceName: "<%= resource_ns_iam -%>_member.foo2",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%> user:[email protected] %s"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>, context["condition_title"]),
ImportState: true,
ImportStateVerify: true,
},
<% end -%>
<% if object.min_version.name == "ga" -%>
{
ResourceName: "<%= resource_ns_iam -%>_member.foo3",
ImportStateId: fmt.Sprintf("<%= import_url -%> <%= object.iam_policy.allowed_iam_role -%> user:[email protected] %s"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>, context["condition_title_no_desc"]),
Expand Down Expand Up @@ -404,7 +394,7 @@ func TestAcc<%= resource_name -%>IamPolicyGenerated_withCondition(t *testing.T)
resource.TestCheckResourceAttrWith("data.google_iam_policy.foo", "policy_data", checkGoogleIamPolicy),
),
},
<% if object.min_version.name == "ga" -%>
<% unless object.iam_policy.skip_import_test -%>
{
ResourceName: "<%= resource_ns_iam -%>_policy.foo",
ImportStateId: fmt.Sprintf("<%= import_url -%>"<% unless import_qualifiers.empty? -%>, <% end -%><%= import_qualifiers.join(', ') -%>, <%= example.primary_resource_name -%>),
Expand Down