Skip to content

Conversation

@Mathew-Estafanous
Copy link
Contributor

@Mathew-Estafanous Mathew-Estafanous commented Sep 4, 2025

What does this PR do?

PR #40 fixed role arn name parsing to account for a role's path. This PR adds unit tests to validate that change/

Motivation

Improve test coverage for edge cases like the one identified in the merged PR.

Describe how you validated your changes

  • CI is green 🟢

Additional Notes

@Mathew-Estafanous Mathew-Estafanous self-assigned this Sep 4, 2025
@Mathew-Estafanous Mathew-Estafanous changed the title Add tests for IAM role names with paths. [CONTP-962] Add tests for IAM role names with paths. Sep 4, 2025
@Mathew-Estafanous Mathew-Estafanous marked this pull request as ready for review September 5, 2025 15:02
@Mathew-Estafanous Mathew-Estafanous requested a review from a team as a code owner September 5, 2025 15:02

taskRoleArn := task["task_role_arn"]
s.NotEmpty(taskRoleArn, "Task role ARN should not be empty")
s.Contains(taskRoleArn, "/terraform-test/", "Task role ARN should contain the path '/test-path/'")
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: expected role path

Suggested change
s.Contains(taskRoleArn, "/terraform-test/", "Task role ARN should contain the path '/test-path/'")
s.Contains(taskRoleArn, "/terraform-test/", "Task role ARN should contain the path '/terraform-test/'")

Copy link
Collaborator

@gabedos gabedos left a comment

Choose a reason for hiding this comment

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

Approved with small comment. Thanks for adding these tests!

@Mathew-Estafanous
Copy link
Contributor Author

/merge

@dd-devflow-routing-codex
Copy link

dd-devflow-routing-codex bot commented Sep 9, 2025

View all feedbacks in Devflow UI.

2025-09-09 19:58:35 UTC ℹ️ Start processing command /merge


2025-09-09 19:58:42 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 0s (p90).


2025-09-09 20:01:08 UTC 🚨 MergeQueue: This merge request is in error

mergequeue build completed successfully, but the github api returned an error while merging the pr

Details

Error: PUT https://api.github.com/repos/DataDog/terraform-aws-ecs-datadog/pulls/42/merge: 405 Merge commits are not allowed on this repository. [] (Request ID: 8C34:89BE4:7FD034:1CE968D:68C0877D)

FullStacktrace:
activity error (type: github.GithubService_MergePullRequest, scheduledEventID: 41, startedEventID: 42, identity: 1@github-worker-7c9ff455b9-2ldml@): PUT https://api.github.com/repos/DataDog/terraform-aws-ecs-datadog/pulls/42/merge: 405 Merge commits are not allowed on this repository. [] (Request ID: 8C34:89BE4:7FD034:1CE968D:68C0877D) (type: GitFailure, retryable: false): PUT https://api.github.com/repos/DataDog/terraform-aws-ecs-datadog/pulls/42/merge: 405 Merge commits are not allowed on this repository. [] (type: ErrorResponse, retryable: true)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants