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

SDK is missing generated structs for IAM Policies #127

Closed
insasho opened this issue Mar 6, 2015 · 12 comments
Closed

SDK is missing generated structs for IAM Policies #127

insasho opened this issue Mar 6, 2015 · 12 comments
Assignees
Labels
feature-request A feature should be added or improved.

Comments

@insasho
Copy link

insasho commented Mar 6, 2015

This is a feature request.

I'd like to algorithmically generate and manage IAM policy documents. The SDK should expose a Policy type so that developers can compose policies using canonical types.

The Java SDK has builders for policies: http://docs.aws.amazon.com/AWSSdkDocsJava/latest//DeveloperGuide/java-dg-access-control.html

Thanks.

@lsegal lsegal added the feature-request A feature should be added or improved. label Mar 6, 2015
@radeksimko
Copy link
Contributor

This would help us a lot in Terraform. We have quite a few outstanding issues being constantly reported due to the fact we're not able to fully parse & reconstruct the IAM JSON structure.

hashicorp/terraform#3634
hashicorp/terraform#3124
hashicorp/terraform#4278
hashicorp/terraform#3519

and many others.

If this doesn't get implemented, we may just end up creating the IAM structs ourselves.

@jasdel
Copy link
Contributor

jasdel commented Jan 25, 2016

@radeksimko Thanks for the heads up on the related issues. Adding modeling of policies to the SDK would be a helpful feature and some of the SDKs provide similar support. I think this would be good to add to the SDK.

I think initially we need to determine and define the definition of the various policy formats. There is a generic base container format for a policy but the internals can vary wildly by service, and some are not necessary compatibile with others. Syncing with how the other SDKs handle policy type definitions will also be helpful.

@radeksimko
Copy link
Contributor

Syncing with how the other SDKs handle policy type definitions will also be helpful.

@jasdel Did you have any chance to sync with other SDK teams?

@jasdel jasdel assigned jimfl and jasdel and unassigned jimfl Mar 25, 2016
@jasdel
Copy link
Contributor

jasdel commented Apr 1, 2016

Hi @radeksimko we are still having continuing discussions about how we'd like to represent the types across our SDKs in a consistent way. Potentially using generation, and models to define the types.

@pmoust
Copy link

pmoust commented May 13, 2016

Hello, just pointing out that this is a blocker for our projects as well.
It would greatly help if this could get in sync with the status of other SDKs.

@jasdel
Copy link
Contributor

jasdel commented May 13, 2016

Thanks for the feedback. This will help us determine which features we implement in the future. Currently Java and to a lesser extend .Net are the only SDKs which support policy generation or typing. Ruby v1 had this feature but it was dropped in v2, due to lack of maintainability. With that said this feature would be helpful and alleviate the confusion and pain points surrounding creating and specifying AIM policies.

We currently do not have a standardized method of annotating, or tracking the nuances of AIM policy customizations that needed for various usages. This information is mostly captured in documentation, but not directly available in an way accessible to code.

Something that would help us determine how to implement this feature would be to get a better idea how users would use the utility, and what they would expect it to provide.

  • Since policies very pretty widely from service to service, is there a specific subset that would be more helpful that others to have?
  • How would you use the utility? Would you expect it to be a type that you set fields on, or more of a builder pattern?

@jasdel jasdel added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label May 13, 2016
@pmoust
Copy link

pmoust commented May 25, 2016

@jasdel anything related to s3/ec2/lamda for us. A builder pattern would be a nice generic solution, that said we'd be happy with just exposed structs as well.

@jasdel jasdel removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 7, 2016
@jasdel
Copy link
Contributor

jasdel commented Jun 7, 2016

Thanks for the update @pmoust. We still have this task in our backlog, and having these discussions will help us build the policy models when that work is started.

@radeksimko
Copy link
Contributor

Since policies very pretty widely from service to service, is there a specific subset that would be more helpful that others to have?

I'd say IAM resources (Users, Roles, Groups, ...) are currently causing most of the pain for Terraform users since these always require/expect the user to define some kind of policy.

Other policies for other services (S3, SNS, Lambda, ECR, ES, SQS, ...) can be optional in many cases (or default to sensible policy which doesn't need to be diff'ed and cause confusions).

This is why I believe that policies for IAM resources should be given priority, but eventually Terraform users/developers would appreciate structs for all policies. 😉

How would you use the utility? Would you expect it to be a type that you set fields on, or more of a builder pattern?

Both marshal & unmarshal - pretty much the same thing we do today for all JSON-based fields in Terraform:

func normalizeJson(jsonString interface{}) string {
    if jsonString == nil || jsonString == "" {
        return ""
    }
    var j interface{}
    err := json.Unmarshal([]byte(jsonString.(string)), &j)
    if err != nil {
        return fmt.Sprintf("Error parsing JSON: %s", err)
    }
    b, _ := json.Marshal(j)
    return string(b[:])
}

No matter what format user used we'd like to be able to construct a canonical version of the policy to figure out if there's any change (during the dry-run) and eventually in the future allow some fancy user-friendly diff-ing too.

Basically we're looking for something like this: https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/iam_policy_model.go in AWS SDK, managed & updated by AWS.

@jasdel
Copy link
Contributor

jasdel commented Jun 28, 2016

Thanks for the extra information @radeksimko this will help us prioritize the SDK's backlog. I agree it makes sense to prioritize the IAM policies over other policies. In addition thanks for linking to the terraform policy model. I think the model you linked is a good place to start for the SDK to start with. Further down the road it would be great if the SDK were able to generate definitions/validators for various service policies, but that information isn't easily available.

Out of curiosity, why is the do does Terraform need to normalize the JSON strings? is this to ensure the string is encoded correct?

@radeksimko
Copy link
Contributor

Out of curiosity, why is the do does Terraform need to normalize the JSON strings? is this to ensure the string is encoded correct?

In simple terms each time user runs terraform plan or terraform apply Terraform compares the real state (returned from Describe*/Get*/... API methods) with definitions in HCL configs to a) provide a diff to the user as part of dry-run functionality and b) decide which part of the infrastructure to update

  • Comparing primitive data types, maps, sorted and unsorted (in cases where AWS returns items in random order) lists is fairly easy
  • Comparing predictable JSON structure as string would be probably work fine, but we don't do it as we want whitespace changes to be noop and prevent noop update API calls. i.e. {"k":"v"} should be treated as equal to { "k" : "v" }
  • Comparing unpredictable JSON structure is impossible without the structs we're requesting here because JSON sent to API != JSON returned back from API. This is why Terraform generates unnecessary diffs today and performs basically noop Update API calls of many IAM policies, purely because the JSON structure (unmarshalled to a simple nested map) doesn't match.

TL;DR Terraform can't tell if "Resource": ["arn:*"] is the same as "Resource": "arn:*". For AWS it's equal.

@github-actions
Copy link

We have noticed this issue has not recieved attention in 3 years. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Apr 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

6 participants