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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

d/aws_iam_session_context: New data source #19957

Merged
merged 29 commits into from
Jun 28, 2021
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d86a246
caller_identity: Add source role
YakDriver Jun 23, 2021
b81de6a
d/caller_identity: Add source_arn
YakDriver Jun 24, 2021
11d04ce
tests/d/caller_identity: Unit test source_arn
YakDriver Jun 24, 2021
c659b64
docs/d/caller_identity: Docs for source_arn
YakDriver Jun 24, 2021
62d7c93
d/caller_identity: Add changelog
YakDriver Jun 24, 2021
984d0cd
docs/d/caller_identity: Add example
YakDriver Jun 24, 2021
4ecde63
tests/d/caller_identity: Appease dear linter
YakDriver Jun 24, 2021
f887951
ds/caller_identity: Revert to previous
YakDriver Jun 25, 2021
ad1f9a3
ds/iam_assumed_role_source: Update changelog
YakDriver Jun 25, 2021
ae63556
i/iam_role: Add role finder
YakDriver Jun 25, 2021
1125604
provider: Add new data source
YakDriver Jun 25, 2021
3353636
d/iam_assumed_role_source: New data source
YakDriver Jun 25, 2021
1cdcdf2
tests/d/iam_assumed_role_source: New data source
YakDriver Jun 25, 2021
8394cf0
docs/d/iam_assumed_role_source: New data source
YakDriver Jun 25, 2021
914cd83
tests/d/iam_assumed_role_source: Remove comments
YakDriver Jun 25, 2021
39f176a
docs/d/caller_identity: Remove old attribute
YakDriver Jun 25, 2021
a4704cc
docs/d/iam_assumed_role_source: Clarify docs
YakDriver Jun 25, 2021
1a02f96
iam: Rename role finder for general use
YakDriver Jun 28, 2021
8abcda5
d/iam_assumed_role: Rework ARN helper function
YakDriver Jun 28, 2021
074ec4e
tests/d/iam_assumed_role: Rework ARN helper function
YakDriver Jun 28, 2021
c096125
d/iam_assumed_role: Add extra check for nil
YakDriver Jun 28, 2021
aa4183c
d/iam_session_context: Refactor
YakDriver Jun 28, 2021
3f2f55b
d/iam_session_context: Refactor attribute
YakDriver Jun 28, 2021
4947200
d/iam_session_context: Update changelog
YakDriver Jun 28, 2021
8c76c24
d/iam_session_context: Refactor arguments, attributes
YakDriver Jun 28, 2021
1232685
docs/d/iam_session_context: Fix docs
YakDriver Jun 28, 2021
db6575b
d/iam_session_context: Rework helper
YakDriver Jun 28, 2021
a824e7c
Update website/docs/d/iam_session_context.markdown
YakDriver Jun 28, 2021
4252de0
Fix docs
YakDriver Jun 28, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/19957.txt
@@ -0,0 +1,3 @@
```release-note:new-data-source
aws_iam_assumed_role_source
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
aws_iam_assumed_role_source
aws_iam_session_context

```
2 changes: 1 addition & 1 deletion aws/data_source_aws_caller_identity.go
Expand Up @@ -39,7 +39,7 @@ func dataSourceAwsCallerIdentityRead(d *schema.ResourceData, meta interface{}) e
res, err := client.GetCallerIdentity(&sts.GetCallerIdentityInput{})

if err != nil {
return fmt.Errorf("Error getting Caller Identity: %w", err)
return fmt.Errorf("getting Caller Identity: %w", err)
}

log.Printf("[DEBUG] Received Caller Identity: %s", res)
Expand Down
140 changes: 140 additions & 0 deletions aws/data_source_aws_iam_assumed_role_source.go
@@ -0,0 +1,140 @@
package aws

import (
"fmt"
"regexp"
"strings"

"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/finder"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
)

func dataSourceAwsIAMAssumedRoleSource() *schema.Resource {
return &schema.Resource{
Read: dataSourceAwsIAMAssumedRoleSourceRead,

Schema: map[string]*schema.Schema{
"arn": {
Type: schema.TypeString,
Required: true,
ewbankkit marked this conversation as resolved.
Show resolved Hide resolved
},
"role_path": {
Type: schema.TypeString,
Computed: true,
},
"role_name": {
Type: schema.TypeString,
Computed: true,
},
"source_arn": {
Type: schema.TypeString,
Computed: true,
},
"session_name": {
Type: schema.TypeString,
Computed: true,
},
},
}
}

func dataSourceAwsIAMAssumedRoleSourceRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).iamconn

arn := d.Get("arn").(string)

d.SetId(arn)

roleName := ""
sessionName := ""
var err error

if roleName, sessionName = roleNameSessionFromARN(arn); roleName == "" {
d.Set("source_arn", arn)
d.Set("session_name", "")
d.Set("role_name", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

The field name structure for source_arn and role_name seems inconsistent. Could we name them both with source? Or we could even return arn, role_arn, role_name and session_name

Copy link
Member Author

Choose a reason for hiding this comment

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

the challenge here is if you're using an IAM user, you should still be able to get out a passed through user ARN from source_arn.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I see what you are saying. Don't love it, but can't think of a better way to do it.

Copy link
Member Author

Choose a reason for hiding this comment

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

issuer_arn?

d.Set("role_path", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think returning role_path is necessary. Seems like an arbitrary field to choose to return from GetRole output. Maybe we could leave it off and if folks want more role context for this datasource we can always add it later?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.


return nil
}

var role *iam.Role

err = resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError {
var err error

role, err = finder.RoleByName(conn, roleName)

if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) {
return resource.RetryableError(err)
}

if err != nil {
return resource.NonRetryableError(err)
}

return nil
})

if tfresource.TimedOut(err) {
role, err = finder.RoleByName(conn, roleName)
}

if err != nil {
return fmt.Errorf("unable to get role (%s): %w", roleName, err)
}

if role == nil || role.Arn == nil {
return fmt.Errorf("empty role returned (%s)", roleName)
}

d.Set("session_name", sessionName)
d.Set("role_name", roleName)
d.Set("source_arn", role.Arn)
d.Set("role_path", role.Path)

return nil
}

// roleNameSessionFromARN returns the role and session names in an ARN if any.
// Otherwise, it returns empty strings.
func roleNameSessionFromARN(rawARN string) (string, string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function would ideally return an error if there is no session name to grab (i.e. its not a valid sts assumed-role arn), right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the problem is that good Go says we should return an error if we don't plan on dealing with it, which we don't.

parsedARN, err := arn.Parse(rawARN)

if err != nil {
return "", ""
}

parts := strings.Split(parsedARN.Resource, "/")

reAssume := regexp.MustCompile(`^assumed-role/.{1,}/.{2,}`)
reRole := regexp.MustCompile(`^role/.{1,}`)

if reAssume.MatchString(parsedARN.Resource) && parsedARN.Service != "sts" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit, IIRC there are enums for the service names? (sts and iam)

Copy link
Member Author

Choose a reason for hiding this comment

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

Can't use the sts enum but can do the iam. Avoiding import of multiple services.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, dependency for just an enum is not worth it. Go principle: little copying is better than a little dependency.

return "", ""
}

if reRole.MatchString(parsedARN.Resource) && parsedARN.Service != "iam" {
Copy link
Contributor

Choose a reason for hiding this comment

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

My take on this is we should spit IAM role ARNs out just like we would any non-sts assumed-role ARN. If they want metadata on a role, they should use the aws_iam_role data source.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we have the role name, I figure I'd also verify IAM role - no cost.

Copy link
Contributor

Choose a reason for hiding this comment

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

Its not costly, but I think it muddies the water a little on what this data source does. Its easiest to say if you pass in an assumed-role arn we'll give you back all of the metadata and if not you only get sourceArn. Once you add in a case for IAM role, I think the use case isn't quite as clear.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I can see that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Water un muddied

return "", ""
}

if !reAssume.MatchString(parsedARN.Resource) && !reRole.MatchString(parsedARN.Resource) {
return "", ""
}

if reRole.MatchString(parsedARN.Resource) && len(parts) > 1 {
return parts[len(parts)-1], ""
}

if len(parts) < 3 {
return "", ""
}

return parts[len(parts)-2], parts[len(parts)-1]
}