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

Fix mistake in policy. Part2 #39

Merged
merged 2 commits into from
Aug 1, 2023
Merged

Fix mistake in policy. Part2 #39

merged 2 commits into from
Aug 1, 2023

Conversation

ramses999
Copy link
Contributor

This is just a continuation of the fix #38.
Prod environment tested. That's how it works correctly.

@ramses999 ramses999 requested review from a team as code owners March 17, 2023 14:59
@ramses999 ramses999 requested review from Gowiem and korenyoni March 17, 2023 14:59
@ramses999
Copy link
Contributor Author

@nitrocode
The previous fix was correct, but not complete. Please commit it soon.

@nitrocode nitrocode added the patch A minor, backward compatible change label Mar 17, 2023
@nitrocode
Copy link
Member

Hmm.. this looks like a revert of PR #38 ...

@nitrocode
Copy link
Member

Please run make pr/auto-format

@ramses999
Copy link
Contributor Author

@nitrocode Pay attention to the file
https://github.com/cloudposse/terraform-aws-cloudwatch-logs/blob/master/iam.tf
It has the same lines 47 and 55.
In the previous commit, we fixed only the 55th line.
The uncorrected line number 47 remains.

@ramses999
Copy link
Contributor Author

@nitrocode Is there anything required from my side?

@nitrocode
Copy link
Member

Yes. Please fix the auto format or it will not pass required checks and thus cannot be merged

@ramses999
Copy link
Contributor Author

@nitrocode Unfortunately, I don't understand your internal tools and I'm not sure that I can spend a lot of time to recreate a test bench for your conditions. When I run this command on my computer I get the following error.

`✗ make pr/auto-format
Starting cloudposse/build-harness:slim-latest
docker run --name build-harness
--rm -it
-e PACKAGES_PREFER_HOST=true
-e AWS_ACCESS_KEY_ID -e AWS_SECRET_ACCESS_KEY -e AWS_SESSION_TOKEN -e TERM -e AWS_PROFILE -e AWS_REGION -e AWS_DEFAULT_PROFILE -e AWS_DEFAULT_REGION -e AWS_CONFIG_FILE -e AWS_SHARED_CREDENTIALS_FILE

-v /Users/stack/work/dev/cloudwatch-dev-clone/terraform-aws-cloudwatch-logs:/host
--workdir /host
--entrypoint /usr/bin/make
cloudposse/build-harness:slim-latest github/update terraform/fmt readme
Unable to find image 'cloudposse/build-harness:slim-latest' locally
slim-latest: Pulling from cloudposse/build-harness
docker: no matching manifest for linux/arm64/v8 in the manifest list entries.
See 'docker run --help'.
make: *** [build-harness/runner] Error 125

➜ ✗ docker pull cloudposse/build-harness:slim-latest
slim-latest: Pulling from cloudposse/build-harness
no matching manifest for linux/arm64/v8 in the manifest list entries
➜ terraform-aws-cloudwatch-logs git:(PolicyMistakeAsteriskPart2) ✗`

@nitrocode
Copy link
Member

Hm maybe try copying the .github directory from build-harness repo directly?

@Nuru
Copy link

Nuru commented Aug 1, 2023

/terratest

@Nuru
Copy link

Nuru commented Aug 1, 2023

/terratest

@Nuru Nuru merged commit f622326 into cloudposse:main Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch A minor, backward compatible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants