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.

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

Jump to bottom

[fix] Verify IAM policy in TestAccSystemGetAWSSNSIAMPolicy_basic #441

Merged
merged 1 commit into from
Feb 8, 2021
Merged

[fix] Verify IAM policy in TestAccSystemGetAWSSNSIAMPolicy_basic #441

merged 1 commit into from
Feb 8, 2021

Conversation

yohei1126
Copy link
Contributor

@yohei1126 yohei1126 commented Feb 3, 2021 •
edited

Test Plan

  • acceptance tests
  • …

References

@yohei1126 yohei1126 requested a review from a team as a code owner February 3, 2021 01:56
@yohei1126 yohei1126 force-pushed the fix_TestAccSystemGetAWSSNSIAMPolicy_basic branch from 177d4cc to 61e8813 Compare February 3, 2021 02:01
@yohei1126
Copy link
Contributor Author

@ryanking @alldoami Would you review this?

@yohei1126 yohei1126 changed the title Make IAM User ARN configurable [fix] Make IAM User ARN configurable Feb 3, 2021
ryanking
ryanking suggested changes Feb 3, 2021
View reviewed changes
Copy link
Contributor

@ryanking ryanking left a comment

Choose a reason for hiding this comment

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

I would prefer that we instead rewrite the test to not care at all about the IAM user. I think the assertion could be as simple as "we got some valid JSON".

@yohei1126 yohei1126 force-pushed the fix_TestAccSystemGetAWSSNSIAMPolicy_basic branch from 61e8813 to 90a5ed3 Compare February 4, 2021 01:52
@yohei1126 yohei1126 changed the title [fix] Make IAM User ARN configurable [fix] Verify IAM policy JSON in TestAccSystemGetAWSSNSIAMPolicy_basic using regex Feb 4, 2021
@yohei1126
Copy link
Contributor Author

@ryanking Changed the test to verify policy JSON using regex. Please review it.

@yohei1126 yohei1126 force-pushed the fix_TestAccSystemGetAWSSNSIAMPolicy_basic branch from 90a5ed3 to 06454a5 Compare February 5, 2021 09:37
@yohei1126
Copy link
Contributor Author

@ryanking Can you review this?

@yohei1126 yohei1126 changed the title [fix] Verify IAM policy JSON in TestAccSystemGetAWSSNSIAMPolicy_basic using regex [fix] Verify IAM policy in TestAccSystemGetAWSSNSIAMPolicy_basic Feb 5, 2021
@ryanking
Copy link
Contributor

ryanking commented Feb 5, 2021

/ok-to-test sha=06454a5

@github-actions
Copy link

github-actions bot commented Feb 5, 2021

Integration tests success for 06454a5

@czimergebot czimergebot merged commit 0147134 into Snowflake-Labs:main Feb 8, 2021
@yohei1126 yohei1126 deleted the fix_TestAccSystemGetAWSSNSIAMPolicy_basic branch February 9, 2021 06:31
gjv9491 pushed a commit to gjv9491/terraform-provider-snowflake that referenced this pull request Mar 19, 2021
@yohei1126
Verify aws_sns_topic_policy_json exists (Snowflake-Labs#441)
f1f332d 
<!-- Feel free to delete comments as you fill this in -->

<!-- summary of changes -->

* AWS IAM User ARN is different for each Snowflake account but IAM user ARN is hardcoded in an acceptance test.
* This PR makes the AWS IAM User ARN configurable by environment variable.
* Fix Snowflake-Labs#439

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested -->
* [x] acceptance tests
<!-- add more below if you think they are relevant -->
* [ ] …

## References
<!-- issues documentation links, etc  -->

*
anton-chekanov pushed a commit to anton-chekanov/terraform-provider-snowflake that referenced this pull request Jan 25, 2022
@yohei1126 @anton-chekanov
Verify aws_sns_topic_policy_json exists (Snowflake-Labs#441)
d1e5070 
<!-- Feel free to delete comments as you fill this in -->

<!-- summary of changes -->

* AWS IAM User ARN is different for each Snowflake account but IAM user ARN is hardcoded in an acceptance test.
* This PR makes the AWS IAM User ARN configurable by environment variable.
* Fix Snowflake-Labs#439

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested -->
* [x] acceptance tests
<!-- add more below if you think they are relevant -->
* [ ] …

## References
<!-- issues documentation links, etc  -->

*
daniepett pushed a commit to daniepett/terraform-provider-snowflake that referenced this pull request Feb 9, 2022
@yohei1126
Verify aws_sns_topic_policy_json exists (Snowflake-Labs#441)
209a270 
<!-- Feel free to delete comments as you fill this in -->

<!-- summary of changes -->

* AWS IAM User ARN is different for each Snowflake account but IAM user ARN is hardcoded in an acceptance test.
* This PR makes the AWS IAM User ARN configurable by environment variable.
* Fix Snowflake-Labs#439

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested -->
* [x] acceptance tests
<!-- add more below if you think they are relevant -->
* [ ] …

## References
<!-- issues documentation links, etc  -->

*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanking ryanking ryanking approved these changes

@edulop91 edulop91 Awaiting requested review from edulop91

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Acceptance test "TestAccSystemGetAWSSNSIAMPolicy_basic" fail on different Snowflake account
4 participants
@yohei1126 @ryanking @alldoami @czimergebot