Allow custom AWS tags in iamrole annotation #88
Conversation
Signed-off-by: Nathan Rosenberg <[email protected]>
Signed-off-by: Nathan Rosenberg <[email protected]>
Signed-off-by: Nathan Rosenberg <[email protected]>
nrosenberg1
force-pushed
the
custom-tags-feat
branch
from
a2f1902 to
9776c7e
Compare
controllers/iamrole_controller.go
Outdated
| @@ -298,6 +298,21 @@ func (r *IamroleReconciler) ConstructCreateIAMRoleInput(ctx context.Context, iam | |||
| tags["Cluster"] = config.Props.ClusterName() | |||
| } | |||
|
|
|||
| // Custom tags value should be a string of comma seperated key/value pairs | |||
| // example annotation: "iamroles.iammanager.keikoproj.io/tags": "key1=value1,key2=value2" | |||
| if customTagsString, ok := iamRole.GetAnnotations()["iamroles.iammanager.keikoproj.io/tags"]; ok { | |||
There was a problem hiding this comment.
There is a common utility function to read the annotation. Please use that
There was a problem hiding this comment.
Sure, will change this
| @@ -168,6 +174,34 @@ func CompareAssumeRolePolicy(ctx context.Context, request string, target string) | |||
| return true | |||
| } | |||
|
|
|||
| //CompareTags compares tags from request and response | |||
| func CompareTags(ctx context.Context, request map[string]string, target []*iam.Tag) bool { | |||
There was a problem hiding this comment.
Does this handle the default tags in the comparison? Default tags(managedBy, Namespace, cluster) is not coming in the request annotation so it is always returns that there is a diff
There was a problem hiding this comment.
This is checking roleInput tags so default tags are included in request map, unit tests validate the same
Sure, I will add this validation |
…t tags from overwrite Signed-off-by: Nathan Rosenberg <[email protected]>
|
There is no travis.yaml file in the repo. I also removed travis web hook just now. Try with a dummy commit and see whether it gets refreshed |
Signed-off-by: Nathan Rosenberg <[email protected]>
|
@mnkg561 looks like dummy commit didn't refresh it, guess I will just administrator merge |
Could you share the solution in high level?
"iammanager.keikoproj.io/tags"with value in format"key1=value1;;key2=value2"and controller will tag AWS role accordinglyCould you share the test results?
