From c8c379953c54e21dc30721bf92a6f5f36e9d74b8 Mon Sep 17 00:00:00 2001
From: nmogulla <mnkg561@gmail.com>
Date: Fri, 6 Mar 2020 15:18:17 -0800
Subject: [PATCH 1/4] trusted role changes
---
Makefile | 23 +-
api/v1alpha1/StringOrStrings.go | 52 ++++
api/v1alpha1/iamrole_types.go | 15 ++
api/v1alpha1/zz_generated.deepcopy.go | 37 +++
.../iammanager.keikoproj.io_iamroles.yaml | 12 +
.../iammanager.keikoproj.io_iamroles.yaml | 12 +
controllers/iamrole_controller.go | 35 +--
docs/Configmap_Properties.md | 3 +-
go.mod | 5 +-
go.sum | 16 ++
hack/iam-manager.yaml | 12 +
hack/iam-manager_with_webhook.yaml | 12 +
internal/config/constants.go | 2 +-
internal/config/properties.go | 18 +-
internal/config/properties_test.go | 2 +-
internal/utils/utils.go | 93 +++++++
internal/utils/utils_test.go | 164 +++++++++++++
pkg/awsapi/iam.go | 37 +++
pkg/awsapi/iam_test.go | 49 ++++
pkg/validation/validate.go | 69 +++++-
pkg/validation/validate_test.go | 232 +++++++++++++++++-
21 files changed, 849 insertions(+), 51 deletions(-)
create mode 100644 api/v1alpha1/StringOrStrings.go
create mode 100644 internal/utils/utils.go
create mode 100644 internal/utils/utils_test.go
diff --git a/Makefile b/Makefile
index 0c95b9f..a0cc766 100644
--- a/Makefile
+++ b/Makefile
@@ -20,7 +20,7 @@ export RESTRICTED_POLICY_RESOURCES=policy-resource
export RESTRICTED_S3_RESOURCES=s3-resource
export AWS_ACCOUNT_ID=123456789012
export AWS_REGION=us-west-2
-export AWS_MASTER_ROLE=
+export TRUST_POLICY_ARN_LIST=arn:aws:iam::123456789012:role/trust_role
export MANAGED_POLICIES=arn:aws:iam::123456789012:policy/SOMETHING
export MANAGED_PERMISSION_BOUNDARY_POLICY=arn:aws:iam::1123456789012:role/iam-manager-permission-boundary
@@ -32,7 +32,7 @@ mock:
done
# Run tests
-test: setup mock generate fmt vet manifests
+test: setup mock generate fmt manifests
go test ./... -coverprofile cover.out
# Build manager binary
@@ -45,28 +45,33 @@ run: generate fmt vet manifests
# Install CRDs into a cluster
install: manifests
- kustomize build config/crd | kubectl apply -f -
+ kustomize build config/crd_no_webhook | kubectl apply -f -
# Deploy controller in the configured Kubernetes cluster in ~/.kube/config
-deploy_with_webhook: manifests
+deploy: manifests
cd config/manager && kustomize edit set image controller=${IMG}
- kustomize build config/default | kubectl apply -f -
+ kustomize build config/default_no_webhook | kubectl apply -f -
+
+# Install CRDs into a cluster
+install_with_webhook: manifests
+ kustomize build config/crd | kubectl apply -f -
# Deploy controller in the configured Kubernetes cluster in ~/.kube/config
-deploy: manifests
+deploy_with_webhook: manifests
cd config/manager && kustomize edit set image controller=${IMG}
- kustomize build config/default_no_webhook | kubectl apply -f -
+ kustomize build config/default | kubectl apply -f -
# updates the full config yaml file
update: manifests
- cd config/manager && kustomize edit set image controller=${IMG}
- kustomize build config/default > hack/iam-manager_with_webhook.yaml
cd config/manager && kustomize edit set image controller=${IMG}
kustomize build config/default_no_webhook > hack/iam-manager.yaml
+ kustomize build config/default > hack/iam-manager_with_webhook.yaml
# Generate manifests e.g. CRD, RBAC etc.
manifests: controller-gen
$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
+ $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd_no_webhook/bases
+
# Run go fmt against code
fmt:
diff --git a/api/v1alpha1/StringOrStrings.go b/api/v1alpha1/StringOrStrings.go
new file mode 100644
index 0000000..412dc76
--- /dev/null
+++ b/api/v1alpha1/StringOrStrings.go
@@ -0,0 +1,52 @@
+package v1alpha1
+
+import "encoding/json"
+
+//StringOrStrings type accepts one string or multiple strings
+// +kubebuilder:object:generate=false
+// +k8s:openapi-gen=true
+type StringOrStrings []string
+
+//MarshalJSON function is a custom implementation of json.Marshal for StringOrStrings
+func (s StringOrStrings) MarshalJSON() ([]byte, error) {
+ if len(s) == 1 {
+ return json.Marshal(s[0])
+ }
+ //I need to convert it to string array
+ // if i use json.Marshal(s) here it is going to go into infinite loop
+ // since json.Marshal for type StringOrStrings are overwritten in this very own method
+ var k []string
+ for _, str := range s {
+ k = append(k, str)
+ }
+ return json.Marshal(k)
+}
+
+//UnmarshalJson function is a custom implementation of json to unmarshal StringOrStrings
+func (s *StringOrStrings) UnmarshalJSON(b []byte) error {
+ //Try to convert to array
+ var strings []string
+ if err := json.Unmarshal(b, &strings); err != nil {
+ //If err, convert it to string and add it to array
+ var str string
+ err = json.Unmarshal(b, &str)
+ if err != nil {
+ return err
+ }
+ strings = []string{str}
+
+ }
+
+ *s = strings
+ return nil
+}
+
+// OpenAPISchemaType is used by the kube-openapi generator when constructing
+// the OpenAPI spec of this type.
+//
+// See: https://github.com/kubernetes/kube-openapi/tree/master/pkg/generators
+func (StringOrStrings) OpenAPISchemaType() []string { return []string{"string"} }
+
+// OpenAPISchemaFormat is used by the kube-openapi generator when constructing
+// the OpenAPI spec of this type.
+func (StringOrStrings) OpenAPISchemaFormat() string { return "string-or-strings" }
diff --git a/api/v1alpha1/iamrole_types.go b/api/v1alpha1/iamrole_types.go
index e877f26..d3f5790 100644
--- a/api/v1alpha1/iamrole_types.go
+++ b/api/v1alpha1/iamrole_types.go
@@ -25,6 +25,8 @@ import (
// IamroleSpec defines the desired state of Iamrole
type IamroleSpec struct {
PolicyDocument PolicyDocument `json:"PolicyDocument"`
+ // +optional
+ TrustPolicy TrustPolicy `json:"TrustPolicy,omitempty"`
}
// +kubebuilder:validation:Required
@@ -65,6 +67,19 @@ type Statement struct {
// +kubebuilder:validation:Enum=Allow;Deny
type Effect string
+//TrustPolicy struct holds principal
+type TrustPolicy struct {
+ Principal Principal `json:"Principal,omitempty"`
+}
+
+//Principal struct holds AWS principal
+type Principal struct {
+ // +optional
+ AWS StringOrStrings `json:"AWS,omitempty"`
+ // +optional
+ Service string `json:"Service,omitempty"`
+}
+
const (
//Allow Policy allows policy
AllowPolicy Effect = "Allow"
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
index bcc8675..6453a85 100644
--- a/api/v1alpha1/zz_generated.deepcopy.go
+++ b/api/v1alpha1/zz_generated.deepcopy.go
@@ -86,6 +86,7 @@ func (in *IamroleList) DeepCopyObject() runtime.Object {
func (in *IamroleSpec) DeepCopyInto(out *IamroleSpec) {
*out = *in
in.PolicyDocument.DeepCopyInto(&out.PolicyDocument)
+ in.TrustPolicy.DeepCopyInto(&out.TrustPolicy)
}
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IamroleSpec.
@@ -135,6 +136,26 @@ func (in *PolicyDocument) DeepCopy() *PolicyDocument {
return out
}
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *Principal) DeepCopyInto(out *Principal) {
+ *out = *in
+ if in.AWS != nil {
+ in, out := &in.AWS, &out.AWS
+ *out = make(StringOrStrings, len(*in))
+ copy(*out, *in)
+ }
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Principal.
+func (in *Principal) DeepCopy() *Principal {
+ if in == nil {
+ return nil
+ }
+ out := new(Principal)
+ in.DeepCopyInto(out)
+ return out
+}
+
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *Statement) DeepCopyInto(out *Statement) {
*out = *in
@@ -159,3 +180,19 @@ func (in *Statement) DeepCopy() *Statement {
in.DeepCopyInto(out)
return out
}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *TrustPolicy) DeepCopyInto(out *TrustPolicy) {
+ *out = *in
+ in.Principal.DeepCopyInto(&out.Principal)
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TrustPolicy.
+func (in *TrustPolicy) DeepCopy() *TrustPolicy {
+ if in == nil {
+ return nil
+ }
+ out := new(TrustPolicy)
+ in.DeepCopyInto(out)
+ return out
+}
diff --git a/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml b/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml
index 8107e1e..784522a 100644
--- a/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml
+++ b/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml
@@ -446,6 +446,18 @@ spec:
required:
- Statement
type: object
+ TrustPolicy:
+ properties:
+ Principal:
+ properties:
+ AWS:
+ items:
+ type: string
+ type: array
+ Service:
+ type: string
+ type: object
+ type: object
required:
- PolicyDocument
type: object
diff --git a/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml b/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml
index 8107e1e..784522a 100644
--- a/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml
+++ b/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml
@@ -446,6 +446,18 @@ spec:
required:
- Statement
type: object
+ TrustPolicy:
+ properties:
+ Principal:
+ properties:
+ AWS:
+ items:
+ type: string
+ type: array
+ Service:
+ type: string
+ type: object
+ type: object
required:
- PolicyDocument
type: object
diff --git a/controllers/iamrole_controller.go b/controllers/iamrole_controller.go
index 672d580..3f17284 100644
--- a/controllers/iamrole_controller.go
+++ b/controllers/iamrole_controller.go
@@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"github.com/keikoproj/iam-manager/internal/config"
+ "github.com/keikoproj/iam-manager/internal/utils"
"github.com/keikoproj/iam-manager/pkg/awsapi"
"github.com/keikoproj/iam-manager/pkg/log"
"github.com/keikoproj/iam-manager/pkg/validation"
@@ -115,19 +116,6 @@ func (r *IamroleReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
return ctrl.Result{}, nil
}
-var roleTrust = `{
- "Version": "2012-10-17",
- "Statement": [
- {
- "Effect": "Allow",
- "Principal": {
- "AWS": "arn:aws:iam::000065563193:role/masters.ops-prim-ppd.cluster.k8s.local"
- },
- "Action": "sts:AssumeRole"
- }
- ]
-}`
-
//HandleReconcile function handles all the reconcile
func (r *IamroleReconciler) HandleReconcile(ctx context.Context, req ctrl.Request, iamRole *iammanagerv1alpha1.Iamrole) (ctrl.Result, error) {
log := log.Logger(ctx, "controllers", "iamrole_controller", "HandleReconcile")
@@ -135,12 +123,18 @@ func (r *IamroleReconciler) HandleReconcile(ctx context.Context, req ctrl.Reques
log.Info("state of the custom resource ", "state", iamRole.Status.State)
role, _ := json.Marshal(iamRole.Spec.PolicyDocument)
roleName := fmt.Sprintf("k8s-%s", iamRole.ObjectMeta.Name)
+
+ trustPolicy, err := utils.GetTrustPolicy(ctx, &iamRole.Spec.TrustPolicy)
+ if err != nil {
+ r.Recorder.Event(iamRole, v1.EventTypeWarning, string(iammanagerv1alpha1.Error), "Unable to create/update iam role due to error "+err.Error())
+ return r.UpdateStatus(ctx, iamRole, iammanagerv1alpha1.IamroleStatus{RoleName: roleName, ErrorDescription: err.Error()}, iammanagerv1alpha1.Error)
+ }
input := awsapi.IAMRoleRequest{
Name: roleName,
PolicyName: config.InlinePolicyName,
Description: "#DO NOT DELETE#. Managed by iam-manager",
SessionDuration: 3600,
- TrustPolicy: roleTrust,
+ TrustPolicy: trustPolicy,
PermissionPolicy: string(role),
ManagedPermissionBoundaryPolicy: config.Props.ManagedPermissionBoundaryPolicy(),
ManagedPolicies: config.Props.ManagedPolicies(),
@@ -162,6 +156,17 @@ func (r *IamroleReconciler) HandleReconcile(ctx context.Context, req ctrl.Reques
// This can be update request or a duplicate Requeue for the previous status change to Ready
// Check with state of the world to figure out if this event is because of status update
+ targetRole, err := r.IAMClient.GetRole(ctx, input)
+ if err != nil {
+ // THIS SHOULD NEVER HAPPEN
+ // Just requeue in case if it happens
+ log.Error(err, "error in verifying the status of the iam role with state of the world")
+ log.Info("retry count error", "count", iamRole.Status.RetryCount)
+ r.Recorder.Event(iamRole, v1.EventTypeWarning, string(iammanagerv1alpha1.Error), "Unable to create/update iam role due to error "+err.Error())
+ return r.UpdateStatus(ctx, iamRole, iammanagerv1alpha1.IamroleStatus{RetryCount: iamRole.Status.RetryCount + 1, RoleName: roleName, ErrorDescription: err.Error()}, iammanagerv1alpha1.Error, 3000)
+
+ }
+
targetPolicy, err := r.IAMClient.GetRolePolicy(ctx, input)
if err != nil {
// THIS SHOULD NEVER HAPPEN
@@ -173,7 +178,7 @@ func (r *IamroleReconciler) HandleReconcile(ctx context.Context, req ctrl.Reques
}
- if validation.ComparePolicy(ctx, input.PermissionPolicy, *targetPolicy) {
+ if validation.CompareRole(ctx, input, targetRole, *targetPolicy) {
log.Info("No change in the incoming policy compare to state of the world(external AWS IAM) policy")
return ctrl.Result{}, nil
}
diff --git a/docs/Configmap_Properties.md b/docs/Configmap_Properties.md
index c3e20e4..91b9fa5 100644
--- a/docs/Configmap_Properties.md
+++ b/docs/Configmap_Properties.md
@@ -10,4 +10,5 @@ This document explains configmap variables.
| iam.managed.permission.boundary.policy| User managed permission boundary policy|k8s-iam-manager-cluster-permission-boundary |Required |
| webhook.enabled | Enable webhhok? | false | Required |
| iam.role.max.limit.per.namespace | Maximum number of roles per namespace | 1 | Required |
-| aws.region | AWS Region | us-west-2 | Required |
\ No newline at end of file
+| aws.region | AWS Region | us-west-2 | Required |
+| iam.default.trust.policy.role.arn.list| Default trust policy role arn list | | Optional |
\ No newline at end of file
diff --git a/go.mod b/go.mod
index 794ccdf..50109c1 100644
--- a/go.mod
+++ b/go.mod
@@ -5,15 +5,14 @@ go 1.12
require (
github.com/aws/aws-sdk-go v1.25.38
github.com/go-logr/logr v0.1.0
- github.com/golang/mock v1.4.0
+ github.com/golang/mock v1.4.1
github.com/onsi/ginkgo v1.6.0
github.com/onsi/gomega v1.4.2
github.com/pborman/uuid v0.0.0-20170612153648-e790cca94e6c
github.com/pkg/errors v0.8.1
github.com/stretchr/testify v1.4.0 // indirect
- golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 // indirect
- golang.org/x/net v0.0.0-20190620200207-3b0461eec859 // indirect
golang.org/x/sys v0.0.0-20190422165155-953cdadca894 // indirect
+ golang.org/x/tools v0.0.0-20200305185322-6a641547f55b // indirect
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127
k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b
k8s.io/apimachinery v0.0.0-20190404173353-6a84e37a896d
diff --git a/go.sum b/go.sum
index c0a2a67..de1d3a4 100644
--- a/go.sum
+++ b/go.sum
@@ -22,6 +22,8 @@ github.com/golang/groupcache v0.0.0-20180513044358-24b0969c4cb7 h1:u4bArs140e9+A
github.com/golang/groupcache v0.0.0-20180513044358-24b0969c4cb7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
github.com/golang/mock v1.4.0 h1:Rd1kQnQu0Hq3qvJppYSG0HtP+f5LPPUiDswTLiEegLg=
github.com/golang/mock v1.4.0/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
+github.com/golang/mock v1.4.1 h1:ocYkMQY5RrXTYgXl7ICpV0IXwlEQGwKIsery4gyXa1U=
+github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs=
github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
@@ -87,16 +89,20 @@ golang.org/x/crypto v0.0.0-20180820150726-614d502a4dac/go.mod h1:6SG95UA2DQfeDnf
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 h1:ObdrDkeb4kJdCP557AjRjq69pTHfNouLtWZG7j9rPN8=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/net v0.0.0-20180906233101-161cd47e91fd h1:nTDtHvHSdCn1m6ITfMRqtOd/9+7a3s8RBNOZ3eYZzJA=
golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859 h1:R/3boaszxrf1GEUWTVDzSKVwLmSJpwZ1yqXm8j0v2QI=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
+golang.org/x/net v0.0.0-20200226121028-0de0cce0169b h1:0mm1VjtFUOIlE1SbDlwjYaDxZVDP2S5ou6y0gSgXHu8=
+golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be h1:vEDujvNQGv4jgYKudGeI/+DAX4Jffq6hpD55MmoEvKs=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e h1:o3PsSEY8E4eXWkXrIP9YJALUkVZqzHJT5DOasTyn8Vs=
golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
@@ -110,6 +116,16 @@ golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 h1:+DCIGbF/swA92ohVg0//6X2I
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/tools v0.0.0-20190425150028-36563e24a262 h1:qsl9y/CJx34tuA7QCPNp86JNJe4spst6Ff8MjvPUdPg=
golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
+golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
+golang.org/x/tools v0.0.0-20200304193943-95d2e580d8eb h1:iKlO7ROJc6SttHKlxzwGytRtBUqX4VARrNTgP2YLX5M=
+golang.org/x/tools v0.0.0-20200304193943-95d2e580d8eb/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
+golang.org/x/tools v0.0.0-20200305140159-d7d444866696 h1:uuiLBSsR+ZDddgZ/2k23Y7FrUNl29gq4sEFcO170R5k=
+golang.org/x/tools v0.0.0-20200305140159-d7d444866696/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
+golang.org/x/tools v0.0.0-20200305185322-6a641547f55b h1:kWwtroURwYKTlrhKtLws/aJ3iuNdVB417e2FCSutkIs=
+golang.org/x/tools v0.0.0-20200305185322-6a641547f55b/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
+golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gomodules.xyz/jsonpatch/v2 v2.0.1 h1:xyiBuvkD2g5n7cYzx6u2sxQvsAy4QJsZFCzGVdzOXZ0=
gomodules.xyz/jsonpatch/v2 v2.0.1/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU=
google.golang.org/appengine v1.1.0 h1:igQkv0AAhEIvTEpD5LIpAfav2eeVO9HBTjvKHVJPRSs=
diff --git a/hack/iam-manager.yaml b/hack/iam-manager.yaml
index a0a7bdd..7985a0b 100644
--- a/hack/iam-manager.yaml
+++ b/hack/iam-manager.yaml
@@ -454,6 +454,18 @@ spec:
required:
- Statement
type: object
+ TrustPolicy:
+ properties:
+ Principal:
+ properties:
+ AWS:
+ items:
+ type: string
+ type: array
+ Service:
+ type: string
+ type: object
+ type: object
required:
- PolicyDocument
type: object
diff --git a/hack/iam-manager_with_webhook.yaml b/hack/iam-manager_with_webhook.yaml
index 93e1d1f..4aaa116 100644
--- a/hack/iam-manager_with_webhook.yaml
+++ b/hack/iam-manager_with_webhook.yaml
@@ -456,6 +456,18 @@ spec:
required:
- Statement
type: object
+ TrustPolicy:
+ properties:
+ Principal:
+ properties:
+ AWS:
+ items:
+ type: string
+ type: array
+ Service:
+ type: string
+ type: object
+ type: object
required:
- PolicyDocument
type: object
diff --git a/internal/config/constants.go b/internal/config/constants.go
index dc41125..1f4b745 100644
--- a/internal/config/constants.go
+++ b/internal/config/constants.go
@@ -29,7 +29,7 @@ const (
propertAWSAccountID = "aws.accountId"
// aws master role
- propertyAwsMasterRole = "aws.MasterRole"
+ propertyDefaultTrustPolicyARNList = "iam.default.trust.policy.role.arn.list"
// user managed policies
propertyManagedPolicies = "iam.managed.policies"
diff --git a/internal/config/properties.go b/internal/config/properties.go
index de284a0..aa54cd8 100644
--- a/internal/config/properties.go
+++ b/internal/config/properties.go
@@ -23,7 +23,7 @@ type Properties struct {
restrictedPolicyResources []string
restrictedS3Resources []string
awsAccountID string
- awsMasterRole string
+ trustPolicyARNs []string
managedPolicies []string
managedPermissionBoundaryPolicy string
awsRegion string
@@ -70,7 +70,7 @@ func LoadProperties(env string, cm ...*v1.ConfigMap) error {
restrictedPolicyResources: strings.Split(os.Getenv("RESTRICTED_POLICY_RESOURCES"), separator),
restrictedS3Resources: strings.Split(os.Getenv("RESTRICTED_S3_RESOURCES"), separator),
awsAccountID: os.Getenv("AWS_ACCOUNT_ID"),
- awsMasterRole: os.Getenv("AWS_MASTER_ROLE"),
+ trustPolicyARNs: strings.Split(os.Getenv("TRUST_POLICY_ARN_LIST"), separator),
managedPolicies: strings.Split(os.Getenv("MANAGED_POLICIES"), separator),
managedPermissionBoundaryPolicy: os.Getenv("MANAGED_PERMISSION_BOUNDARY_POLICY"),
awsRegion: os.Getenv("AWS_REGION"),
@@ -87,13 +87,11 @@ func LoadProperties(env string, cm ...*v1.ConfigMap) error {
allowedPolicyAction := strings.Split(cm[0].Data[propertyIamPolicyWhitelist], separator)
restrictedPolicyResources := strings.Split(cm[0].Data[propertyIamPolicyBlacklist], separator)
restrictedS3Resources := strings.Split(cm[0].Data[propertyIamPolicyS3Restricted], separator)
- awsMasterRole := cm[0].Data[propertyAwsMasterRole]
Props = &Properties{
allowedPolicyAction: allowedPolicyAction,
restrictedPolicyResources: restrictedPolicyResources,
restrictedS3Resources: restrictedS3Resources,
- awsMasterRole: awsMasterRole,
}
//Defaults
@@ -154,6 +152,14 @@ func LoadProperties(env string, cm ...*v1.ConfigMap) error {
}
Props.managedPolicies = managedPolicies
+ trustPolicyARNs := strings.Split(cm[0].Data[propertyDefaultTrustPolicyARNList], separator)
+ for i := range trustPolicyARNs {
+ if !strings.HasPrefix(trustPolicyARNs[i], "arn:aws:iam::") {
+ trustPolicyARNs[i] = fmt.Sprintf(PolicyARNFormat, awsAccountID, trustPolicyARNs[i])
+ }
+ }
+ Props.trustPolicyARNs = trustPolicyARNs
+
return nil
}
@@ -177,8 +183,8 @@ func (p *Properties) AWSAccountID() string {
return p.awsAccountID
}
-func (p *Properties) AWSMasterRole() string {
- return p.awsMasterRole
+func (p *Properties) TrustPolicyARNs() []string {
+ return p.trustPolicyARNs
}
func (p *Properties) ManagedPermissionBoundaryPolicy() string {
diff --git a/internal/config/properties_test.go b/internal/config/properties_test.go
index 14c0679..0671262 100644
--- a/internal/config/properties_test.go
+++ b/internal/config/properties_test.go
@@ -122,7 +122,7 @@ func (s *PropertiesSuite) TestGetAWSRegion(c *check.C) {
}
func (s *PropertiesSuite) TestGetAWSMasterRole(c *check.C) {
- value := Props.AWSMasterRole()
+ value := Props.TrustPolicyARNs()
c.Assert(value, check.NotNil)
}
diff --git a/internal/utils/utils.go b/internal/utils/utils.go
new file mode 100644
index 0000000..6e3de0e
--- /dev/null
+++ b/internal/utils/utils.go
@@ -0,0 +1,93 @@
+package utils
+
+import (
+ "context"
+ "encoding/json"
+ "errors"
+ "fmt"
+ iammanagerv1alpha1 "github.com/keikoproj/iam-manager/api/v1alpha1"
+ "github.com/keikoproj/iam-manager/internal/config"
+ "github.com/keikoproj/iam-manager/pkg/log"
+ "strings"
+)
+
+//TrustPolicy struct
+type TrustPolicy struct {
+ Version string `json:"Version"`
+ Statement []Statement `json:"Statement"`
+}
+
+//Statement struct
+type Statement struct {
+ Effect Effect `json:"Effect"`
+ Action string `json:"Action"`
+ Principal iammanagerv1alpha1.Principal
+}
+
+// Effect describes whether to allow or deny the specific action
+// Allowed values are
+// - "Allow" : allows the specific action on resources
+// - "Deny" : denies the specific action on resources
+// +kubebuilder:validation:Enum=Allow;Deny
+type Effect string
+
+//Lets use template
+
+//GetTrustPolicy constructs trust policy
+func GetTrustPolicy(ctx context.Context, tPolicy *iammanagerv1alpha1.TrustPolicy) (string, error) {
+ log := log.Logger(ctx, "internal.utils.utils", "GetTrustPolicy")
+ trustPolicy := &TrustPolicy{
+ Version: "2012-10-17",
+ Statement: []Statement{
+ {
+ Effect: "Allow",
+ Action: "sts:AssumeRole",
+ },
+ },
+ }
+ //Same trust policy for all the roles use case
+ //Retrieve the trust policy role arn from config map
+ if tPolicy == nil || (len(tPolicy.Principal.AWS) == 0 && tPolicy.Principal.Service == "") {
+ if len(config.Props.TrustPolicyARNs()) < 1 {
+ msg := "default trust policy is not provided in the config map. Request must provide trust policy in the CR"
+ err := errors.New(msg)
+ log.Error(err, msg)
+ return "", err
+ }
+ var aws []string
+ for _, arn := range config.Props.TrustPolicyARNs() {
+ aws = append(aws, arn)
+ }
+ trustPolicy.Statement[0].Principal.AWS = aws
+ } else {
+ if len(tPolicy.Principal.AWS) != 0 {
+ var aws []string
+ for _, arn := range tPolicy.Principal.AWS {
+ aws = append(aws, arn)
+ }
+ trustPolicy.Statement[0].Principal.AWS = aws
+ }
+ if tPolicy.Principal.Service != "" {
+ //May be lets validate that service must end with .amazonaws.com
+ if !strings.HasSuffix(tPolicy.Principal.Service, "amazonaws.com") {
+ msg := fmt.Sprintf("service %s must end with amazonaws.com in TrustPolicy", tPolicy.Principal.Service)
+ err := errors.New(msg)
+ log.Error(err, msg)
+ return "", err
+ }
+ trustPolicy.Statement[0].Principal.Service = tPolicy.Principal.Service
+ }
+ }
+
+ //Convert it to string
+
+ output, err := json.Marshal(trustPolicy)
+ if err != nil {
+ msg := fmt.Sprintf("malformed trust policy document. unable to marshal it, err = %s", err.Error())
+ err := errors.New(msg)
+ log.Error(err, msg)
+ return "", err
+ }
+ log.V(1).Info("trust policy generated successfully", "trust_policy", string(output))
+ return string(output), nil
+}
diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go
new file mode 100644
index 0000000..99f829b
--- /dev/null
+++ b/internal/utils/utils_test.go
@@ -0,0 +1,164 @@
+package utils_test
+
+import (
+ "context"
+ "encoding/json"
+ "github.com/golang/mock/gomock"
+ "github.com/keikoproj/iam-manager/api/v1alpha1"
+ "github.com/keikoproj/iam-manager/internal/utils"
+ "gopkg.in/check.v1"
+ "testing"
+)
+
+type UtilsTestSuite struct {
+ t *testing.T
+ ctx context.Context
+ mockCtrl *gomock.Controller
+}
+
+func TestValidateSuite(t *testing.T) {
+ check.Suite(&UtilsTestSuite{t: t})
+ check.TestingT(t)
+}
+
+func (s *UtilsTestSuite) SetUpTest(c *check.C) {
+ s.ctx = context.Background()
+ s.mockCtrl = gomock.NewController(s.t)
+}
+
+func (s *UtilsTestSuite) TearDownTest(c *check.C) {
+ s.mockCtrl.Finish()
+}
+
+func (s *UtilsTestSuite) TestGetTrustPolicyDefaultRole(c *check.C) {
+ expect := &utils.TrustPolicy{
+ Version: "2012-10-17",
+ Statement: []utils.Statement{
+ {
+ Effect: "Allow",
+ Action: "sts:AssumeRole",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/trust_role"},
+ },
+ },
+ },
+ }
+
+ expected, _ := json.Marshal(expect)
+ resp, err := utils.GetTrustPolicy(s.ctx, nil)
+ c.Assert(err, check.IsNil)
+ c.Assert(resp, check.DeepEquals, string(expected))
+}
+
+func (s *UtilsTestSuite) TestGetTrustPolicyAWSRoleSuccess(c *check.C) {
+ expect := &utils.TrustPolicy{
+ Version: "2012-10-17",
+ Statement: []utils.Statement{
+ {
+ Effect: "Allow",
+ Action: "sts:AssumeRole",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role"},
+ },
+ },
+ },
+ }
+
+ expected, _ := json.Marshal(expect)
+ input := &v1alpha1.TrustPolicy{
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role"},
+ },
+ }
+ resp, err := utils.GetTrustPolicy(s.ctx, input)
+ c.Assert(err, check.IsNil)
+ c.Assert(resp, check.DeepEquals, string(expected))
+}
+
+func (s *UtilsTestSuite) TestGetTrustPolicyAWSRolesSuccess(c *check.C) {
+ expect := &utils.TrustPolicy{
+ Version: "2012-10-17",
+ Statement: []utils.Statement{
+ {
+ Effect: "Allow",
+ Action: "sts:AssumeRole",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role1", "arn:aws:iam::123456789012:role/user_request_role2"},
+ },
+ },
+ },
+ }
+
+ expected, _ := json.Marshal(expect)
+ input := &v1alpha1.TrustPolicy{
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role1", "arn:aws:iam::123456789012:role/user_request_role2"},
+ },
+ }
+ resp, err := utils.GetTrustPolicy(s.ctx, input)
+ c.Assert(err, check.IsNil)
+ c.Assert(resp, check.DeepEquals, string(expected))
+}
+
+func (s *UtilsTestSuite) TestGetTrustPolicyServiceRoleSuccess(c *check.C) {
+ expect := &utils.TrustPolicy{
+ Version: "2012-10-17",
+ Statement: []utils.Statement{
+ {
+ Effect: "Allow",
+ Action: "sts:AssumeRole",
+ Principal: v1alpha1.Principal{
+ Service: "ec2.amazonaws.com",
+ },
+ },
+ },
+ }
+
+ expected, _ := json.Marshal(expect)
+ input := &v1alpha1.TrustPolicy{
+ Principal: v1alpha1.Principal{
+ Service: "ec2.amazonaws.com",
+ },
+ }
+ resp, err := utils.GetTrustPolicy(s.ctx, input)
+ c.Assert(err, check.IsNil)
+ c.Assert(resp, check.DeepEquals, string(expected))
+}
+
+func (s *UtilsTestSuite) TestGetTrustPolicyAWSRolesAndServiceRoleSuccess(c *check.C) {
+ expect := &utils.TrustPolicy{
+ Version: "2012-10-17",
+ Statement: []utils.Statement{
+ {
+ Effect: "Allow",
+ Action: "sts:AssumeRole",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role1", "arn:aws:iam::123456789012:role/user_request_role2"},
+ Service: "ec2.amazonaws.com",
+ },
+ },
+ },
+ }
+
+ expected, _ := json.Marshal(expect)
+ input := &v1alpha1.TrustPolicy{
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role1", "arn:aws:iam::123456789012:role/user_request_role2"},
+ Service: "ec2.amazonaws.com",
+ },
+ }
+ resp, err := utils.GetTrustPolicy(s.ctx, input)
+ c.Assert(err, check.IsNil)
+ c.Assert(resp, check.DeepEquals, string(expected))
+}
+
+func (s *UtilsTestSuite) TestGetTrustPolicyServiceRoleInvalidName(c *check.C) {
+
+ input := &v1alpha1.TrustPolicy{
+ Principal: v1alpha1.Principal{
+ Service: "ec2.amazonws.com",
+ },
+ }
+ _, err := utils.GetTrustPolicy(s.ctx, input)
+ c.Assert(err, check.NotNil)
+}
diff --git a/pkg/awsapi/iam.go b/pkg/awsapi/iam.go
index 3c73d45..e67e2a0 100644
--- a/pkg/awsapi/iam.go
+++ b/pkg/awsapi/iam.go
@@ -335,6 +335,43 @@ func (i *IAM) AttachInlineRolePolicy(ctx context.Context, req IAMRoleRequest) (*
return &IAMRoleResponse{}, nil
}
+//GetRolePolicy gets the role from aws iam
+func (i *IAM) GetRole(ctx context.Context, req IAMRoleRequest) (*iam.GetRoleOutput, error) {
+ log := log.Logger(ctx, "awsapi", "iam", "GetRole")
+ log.WithValues("roleName", req.Name)
+ log.V(1).Info("Initiating api call")
+ // First get the iam role policy on the AWS IAM side
+ input := &iam.GetRoleInput{
+ RoleName: aws.String(req.Name),
+ }
+
+ if err := input.Validate(); err != nil {
+ log.Error(err, "input validation failed")
+ //should log the error
+ return nil, err
+ }
+
+ resp, err := i.Client.GetRole(input)
+
+ if err != nil {
+ if aerr, ok := err.(awserr.Error); ok {
+ switch aerr.Code() {
+ case iam.ErrCodeNoSuchEntityException:
+ log.Error(err, iam.ErrCodeNoSuchEntityException)
+ case iam.ErrCodeServiceFailureException:
+ log.Error(err, iam.ErrCodeServiceFailureException)
+ default:
+ log.Error(err, aerr.Error())
+ }
+ }
+
+ return nil, err
+ }
+ log.V(1).Info("Successfully able to get the role")
+
+ return resp, nil
+}
+
//GetRolePolicy gets the role from aws iam
func (i *IAM) GetRolePolicy(ctx context.Context, req IAMRoleRequest) (*string, error) {
log := log.Logger(ctx, "awsapi", "iam", "GetRolePolicy")
diff --git a/pkg/awsapi/iam_test.go b/pkg/awsapi/iam_test.go
index 0629c45..903d633 100644
--- a/pkg/awsapi/iam_test.go
+++ b/pkg/awsapi/iam_test.go
@@ -473,6 +473,55 @@ func (s *IAMAPISuite) TestGetRolePolicyFailureUnmodififiablePolicyDocument(c *ch
c.Assert(err, check.NotNil)
}
+//###########
+
+func (s *IAMAPISuite) TestGetRoleSuccess(c *check.C) {
+ s.mockI.EXPECT().GetRole(&iam.GetRoleInput{RoleName: aws.String("VALID_ROLE")}).Times(1).Return(&iam.GetRoleOutput{}, nil)
+ req := awsapi.IAMRoleRequest{Name: "VALID_ROLE", PolicyName: "VALID_POLICY", PermissionPolicy: "SOMETHING"}
+ _, err := s.mockIAM.GetRole(s.ctx, req)
+ c.Assert(err, check.IsNil)
+}
+
+func (s *IAMAPISuite) TestGetRoleInvalidRequest(c *check.C) {
+ req := awsapi.IAMRoleRequest{PermissionPolicy: "SOMETHING"}
+ _, err := s.mockIAM.GetRole(s.ctx, req)
+ c.Assert(err, check.NotNil)
+}
+
+func (s *IAMAPISuite) TestGetRoleFailureMalformedPolicyDocument(c *check.C) {
+ s.mockI.EXPECT().GetRole(&iam.GetRoleInput{RoleName: aws.String("MALFORMED_POLICY")}).Times(1).Return(nil, awserr.New(iam.ErrCodeMalformedPolicyDocumentException, "", errors.New(iam.ErrCodeMalformedPolicyDocumentException)))
+ req := awsapi.IAMRoleRequest{Name: "MALFORMED_POLICY", PolicyName: "VALID_POLICY", PermissionPolicy: "SOMETHING"}
+ _, err := s.mockIAM.GetRole(s.ctx, req)
+ c.Assert(err, check.NotNil)
+}
+
+func (s *IAMAPISuite) TestGetRoleFailureLimitExceeded(c *check.C) {
+ s.mockI.EXPECT().GetRole(&iam.GetRoleInput{RoleName: aws.String("TOO_MANY_REQUEST")}).Times(1).Return(nil, awserr.New(iam.ErrCodeLimitExceededException, "", errors.New(iam.ErrCodeLimitExceededException)))
+ req := awsapi.IAMRoleRequest{Name: "TOO_MANY_REQUEST", PolicyName: "VALID_POLICY", PermissionPolicy: "SOMETHING"}
+ _, err := s.mockIAM.GetRole(s.ctx, req)
+ c.Assert(err, check.NotNil)
+}
+
+func (s *IAMAPISuite) TestGetRoleFailureNoSuchEntity(c *check.C) {
+ s.mockI.EXPECT().GetRole(&iam.GetRoleInput{RoleName: aws.String("NO_SUCH_ENTITY")}).Times(1).Return(nil, awserr.New(iam.ErrCodeNoSuchEntityException, "", errors.New(iam.ErrCodeNoSuchEntityException)))
+ req := awsapi.IAMRoleRequest{Name: "NO_SUCH_ENTITY", PolicyName: "VALID_POLICY", PermissionPolicy: "SOMETHING"}
+ _, err := s.mockIAM.GetRole(s.ctx, req)
+ c.Assert(err, check.NotNil)
+}
+func (s *IAMAPISuite) TestGetRoleFailureServiceFailure(c *check.C) {
+ s.mockI.EXPECT().GetRole(&iam.GetRoleInput{RoleName: aws.String("SERVICE_FAILURE")}).Times(1).Return(nil, awserr.New(iam.ErrCodeServiceFailureException, "", errors.New(iam.ErrCodeServiceFailureException)))
+ req := awsapi.IAMRoleRequest{Name: "SERVICE_FAILURE", PolicyName: "VALID_POLICY", PermissionPolicy: "SOMETHING"}
+ _, err := s.mockIAM.GetRole(s.ctx, req)
+ c.Assert(err, check.NotNil)
+}
+
+func (s *IAMAPISuite) TestGetRoleFailureUnmodififiablePolicyDocument(c *check.C) {
+ s.mockI.EXPECT().GetRole(&iam.GetRoleInput{RoleName: aws.String("UNMODIFIABLE_POLICY")}).Times(1).Return(nil, awserr.New(iam.ErrCodeUnmodifiableEntityException, "", errors.New(iam.ErrCodeUnmodifiableEntityException)))
+ req := awsapi.IAMRoleRequest{Name: "UNMODIFIABLE_POLICY", PolicyName: "VALID_POLICY", PermissionPolicy: "SOMETHING"}
+ _, err := s.mockIAM.GetRole(s.ctx, req)
+ c.Assert(err, check.NotNil)
+}
+
//############################
func (s *IAMAPISuite) TestAttachManagedRolePolicySuccess(c *check.C) {
diff --git a/pkg/validation/validate.go b/pkg/validation/validate.go
index 05972e1..78dfdba 100644
--- a/pkg/validation/validate.go
+++ b/pkg/validation/validate.go
@@ -4,8 +4,11 @@ import (
"context"
"encoding/json"
"fmt"
+ "github.com/aws/aws-sdk-go/service/iam"
"github.com/keikoproj/iam-manager/api/v1alpha1"
"github.com/keikoproj/iam-manager/internal/config"
+ "github.com/keikoproj/iam-manager/internal/utils"
+ "github.com/keikoproj/iam-manager/pkg/awsapi"
"github.com/keikoproj/iam-manager/pkg/log"
"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/util/validation/field"
@@ -86,28 +89,76 @@ func ValidateIAMPolicyResource(ctx context.Context, pDoc v1alpha1.PolicyDocument
return nil
}
-//ComparePolicy function compares input policy doc to target policy doc
-func ComparePolicy(ctx context.Context, request string, target string) bool {
+//CompareRole function compares input role to target role
+func CompareRole(ctx context.Context, request awsapi.IAMRoleRequest, role *iam.GetRoleOutput, target string) bool {
log := log.Logger(ctx, "pkg.validation", "ComparePolicy")
+ // Step 1: Compare the permission policy
+ if !ComparePermissionPolicy(ctx, request, target) {
+ return false
+ }
+
+ //Step 2: Compare Assume Role Policy Document
+ if !CompareAssumeRolePolicy(ctx, request, role) {
+ return false
+ }
+ //Step 3: Compare Permission Boundary
+ if !reflect.DeepEqual(request.ManagedPermissionBoundaryPolicy, *role.Role.PermissionsBoundary.PermissionsBoundaryArn) {
+ log.Info("input permission boundary and target permission boundary are NOT equal")
+ return false
+ }
+
+ return true
+}
+
+//ComparePermissionPolicy compares assume role policy from request and response
+func ComparePermissionPolicy(ctx context.Context, request awsapi.IAMRoleRequest, target string) bool {
+ log := log.Logger(ctx, "pkg.validation", "CompareAssumeRolePolicy")
+
d, _ := url.QueryUnescape(target)
dest := v1alpha1.PolicyDocument{}
err := json.Unmarshal([]byte(d), &dest)
if err != nil {
- log.Error(err, "failed to unmarshal policy document", target)
+ log.Error(err, "failed to unmarshal policy document")
}
req := v1alpha1.PolicyDocument{}
- err = json.Unmarshal([]byte(request), &req)
+ err = json.Unmarshal([]byte(request.PermissionPolicy), &req)
if err != nil {
- log.Error(err, "failed to marshal policy document", request)
+ log.Error(err, "failed to marshal policy document")
}
//compare
- if reflect.DeepEqual(req, dest) {
- log.Info("input policy and target policy are equal")
- return true
+ if !reflect.DeepEqual(req, dest) {
+ log.Info("input policy and target policy are NOT equal")
+ return false
}
- return false
+
+ return true
+}
+
+//CompareAssumeRolePolicy compares assume role policy from request and response
+func CompareAssumeRolePolicy(ctx context.Context, request awsapi.IAMRoleRequest, role *iam.GetRoleOutput) bool {
+ log := log.Logger(ctx, "pkg.validation", "CompareAssumeRolePolicy")
+
+ a, _ := url.QueryUnescape(*role.Role.AssumeRolePolicyDocument)
+ destAssume := utils.TrustPolicy{}
+ err := json.Unmarshal([]byte(a), &destAssume)
+ if err != nil {
+ log.Error(err, "failed to unmarshal assume role policy document")
+ }
+
+ reqAssume := utils.TrustPolicy{}
+ err = json.Unmarshal([]byte(request.TrustPolicy), &reqAssume)
+ if err != nil {
+ log.Error(err, "failed to marshal assume role policy document")
+ }
+ //compare
+ if !reflect.DeepEqual(reqAssume, destAssume) {
+ log.Info("input assume role policy and target assume role policy are NOT equal")
+ return false
+ }
+
+ return true
}
//ContainsString Helper functions to check from a slice of strings.
diff --git a/pkg/validation/validate_test.go b/pkg/validation/validate_test.go
index 06740a9..37ab365 100644
--- a/pkg/validation/validate_test.go
+++ b/pkg/validation/validate_test.go
@@ -3,8 +3,12 @@ package validation_test
import (
"context"
"encoding/json"
+ "github.com/aws/aws-sdk-go/service/iam"
"github.com/golang/mock/gomock"
"github.com/keikoproj/iam-manager/api/v1alpha1"
+ "github.com/keikoproj/iam-manager/internal/config"
+ "github.com/keikoproj/iam-manager/internal/utils"
+ "github.com/keikoproj/iam-manager/pkg/awsapi"
"github.com/keikoproj/iam-manager/pkg/validation"
"gopkg.in/check.v1"
"testing"
@@ -86,7 +90,80 @@ func (s *ValidateSuite) TestValidateIAMPolicyResourceFailure(c *check.C) {
c.Assert(err, check.NotNil)
}
-func (s *ValidateSuite) TestComparePolicySuccess(c *check.C) {
+func (s *ValidateSuite) TestCompareRoleSuccess(c *check.C) {
+
+ input1 := v1alpha1.PolicyDocument{
+ Statement: []v1alpha1.Statement{
+ {
+ Action: []string{"route53:Get"},
+ Effect: "Allow",
+ Resource: []string{"policy-resource"},
+ },
+ },
+ }
+ input2 := v1alpha1.PolicyDocument{
+ Statement: []v1alpha1.Statement{
+ {
+ Action: []string{"route53:Get"},
+ Effect: "Allow",
+ Resource: []string{"policy-resource"},
+ },
+ },
+ }
+
+ role1, _ := json.Marshal(input1)
+ role2, _ := json.Marshal(input2)
+
+ input3 := utils.TrustPolicy{
+ Statement: []utils.Statement{
+ {
+ Action: "route53:Get",
+ Effect: "Allow",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role"},
+ },
+ },
+ },
+ Version: "2012-10-17",
+ }
+
+ input4 := utils.TrustPolicy{
+ Statement: []utils.Statement{
+ {
+ Action: "route53:Get",
+ Effect: "Allow",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role"},
+ },
+ },
+ },
+ Version: "2012-10-17",
+ }
+ role3, _ := json.Marshal(input3)
+ role4, _ := json.Marshal(input4)
+ doc := string(role4)
+ boundary := config.Props.ManagedPermissionBoundaryPolicy()
+ target := iam.GetRoleOutput{
+ Role: &iam.Role{
+ AssumeRolePolicyDocument: &doc,
+ PermissionsBoundary: &iam.AttachedPermissionsBoundary{
+ PermissionsBoundaryArn: &boundary,
+ },
+ },
+ }
+
+ i1 := awsapi.IAMRoleRequest{
+ PermissionPolicy: string(role1),
+ TrustPolicy: string(role3),
+ ManagedPermissionBoundaryPolicy: config.Props.ManagedPermissionBoundaryPolicy(),
+ }
+
+ flag := validation.CompareRole(s.ctx, i1, &target, string(role2))
+ c.Assert(flag, check.Equals, true)
+}
+
+func (s *ValidateSuite) TestComparePermissionPolicySuccess(c *check.C) {
+
input1 := v1alpha1.PolicyDocument{
Statement: []v1alpha1.Statement{
{
@@ -108,11 +185,16 @@ func (s *ValidateSuite) TestComparePolicySuccess(c *check.C) {
role1, _ := json.Marshal(input1)
role2, _ := json.Marshal(input2)
- flag := validation.ComparePolicy(s.ctx, string(role1), string(role2))
+
+ i1 := awsapi.IAMRoleRequest{
+ PermissionPolicy: string(role1),
+ }
+
+ flag := validation.ComparePermissionPolicy(s.ctx, i1, string(role2))
c.Assert(flag, check.Equals, true)
}
-func (s *ValidateSuite) TestComparePolicy2Success(c *check.C) {
+func (s *ValidateSuite) TestComparePermissionPolicy2Success(c *check.C) {
input1 := v1alpha1.PolicyDocument{
Statement: []v1alpha1.Statement{
{
@@ -134,11 +216,15 @@ func (s *ValidateSuite) TestComparePolicy2Success(c *check.C) {
role1, _ := json.Marshal(input1)
role2, _ := json.Marshal(input2)
- flag := validation.ComparePolicy(s.ctx, string(role1), string(role2))
+ i1 := awsapi.IAMRoleRequest{
+ PermissionPolicy: string(role1),
+ }
+
+ flag := validation.ComparePermissionPolicy(s.ctx, i1, string(role2))
c.Assert(flag, check.Equals, true)
}
-func (s *ValidateSuite) TestComparePolicyFailure(c *check.C) {
+func (s *ValidateSuite) TestComparePermissionPolicyFailure(c *check.C) {
input1 := v1alpha1.PolicyDocument{
Statement: []v1alpha1.Statement{
{
@@ -160,7 +246,141 @@ func (s *ValidateSuite) TestComparePolicyFailure(c *check.C) {
role1, _ := json.Marshal(input1)
role2, _ := json.Marshal(input2)
- flag := validation.ComparePolicy(s.ctx, string(role1), string(role2))
+ i1 := awsapi.IAMRoleRequest{
+ PermissionPolicy: string(role1),
+ }
+
+ flag := validation.ComparePermissionPolicy(s.ctx, i1, string(role2))
+ c.Assert(flag, check.Equals, false)
+}
+
+func (s *ValidateSuite) TestCompareAssumeRolePolicySuccess(c *check.C) {
+
+ input1 := utils.TrustPolicy{
+ Statement: []utils.Statement{
+ {
+ Action: "route53:Get",
+ Effect: "Allow",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role"},
+ },
+ },
+ },
+ Version: "2012-10-17",
+ }
+
+ input2 := utils.TrustPolicy{
+ Statement: []utils.Statement{
+ {
+ Action: "route53:Get",
+ Effect: "Allow",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role"},
+ },
+ },
+ },
+ Version: "2012-10-17",
+ }
+ role1, _ := json.Marshal(input1)
+ role2, _ := json.Marshal(input2)
+ doc := string(role2)
+ target := iam.GetRoleOutput{
+ Role: &iam.Role{
+ AssumeRolePolicyDocument: &doc,
+ },
+ }
+
+ i1 := awsapi.IAMRoleRequest{
+ TrustPolicy: string(role1),
+ }
+
+ flag := validation.CompareAssumeRolePolicy(s.ctx, i1, &target)
+ c.Assert(flag, check.Equals, true)
+}
+
+func (s *ValidateSuite) TestCompareAssumeRolePolicy2Success(c *check.C) {
+ input1 := utils.TrustPolicy{
+ Statement: []utils.Statement{
+ {
+ Action: "route53:Get",
+ Effect: "Allow",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role"},
+ },
+ },
+ },
+ Version: "2012-10-17",
+ }
+
+ input2 := utils.TrustPolicy{
+ Statement: []utils.Statement{
+ {
+ Effect: "Allow",
+ Action: "route53:Get",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role"},
+ },
+ },
+ },
+ Version: "2012-10-17",
+ }
+ role1, _ := json.Marshal(input1)
+ role2, _ := json.Marshal(input2)
+ doc := string(role2)
+ target := iam.GetRoleOutput{
+ Role: &iam.Role{
+ AssumeRolePolicyDocument: &doc,
+ },
+ }
+
+ i1 := awsapi.IAMRoleRequest{
+ TrustPolicy: string(role1),
+ }
+
+ flag := validation.CompareAssumeRolePolicy(s.ctx, i1, &target)
+ c.Assert(flag, check.Equals, true)
+}
+
+func (s *ValidateSuite) TestCompareAssumeRolePolicyFailure(c *check.C) {
+ input1 := utils.TrustPolicy{
+ Statement: []utils.Statement{
+ {
+ Action: "route53:Get",
+ Effect: "Allow",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role"},
+ },
+ },
+ },
+ Version: "2012-10-17",
+ }
+
+ input2 := utils.TrustPolicy{
+ Statement: []utils.Statement{
+ {
+ Effect: "Deny",
+ Action: "route53:Get",
+ Principal: v1alpha1.Principal{
+ AWS: []string{"arn:aws:iam::123456789012:role/user_request_role"},
+ },
+ },
+ },
+ Version: "2012-10-17",
+ }
+ role1, _ := json.Marshal(input1)
+ role2, _ := json.Marshal(input2)
+ doc := string(role2)
+ target := iam.GetRoleOutput{
+ Role: &iam.Role{
+ AssumeRolePolicyDocument: &doc,
+ },
+ }
+
+ i1 := awsapi.IAMRoleRequest{
+ TrustPolicy: string(role1),
+ }
+
+ flag := validation.CompareAssumeRolePolicy(s.ctx, i1, &target)
c.Assert(flag, check.Equals, false)
}
From 6e63f12bdcb39e3d6c68b67f7065b2afffffec46 Mon Sep 17 00:00:00 2001
From: nmogulla <mnkg561@gmail.com>
Date: Fri, 6 Mar 2020 15:56:25 -0800
Subject: [PATCH 2/4] fixing string bug too
---
api/v1alpha1/StringOrStrings.go | 13 ++-----------
.../iammanager.keikoproj.io_iamroles.yaml | 3 ++-
.../iammanager.keikoproj.io_iamroles.yaml | 3 ++-
config/crd_no_webhook/kustomization.yaml | 2 +-
hack/iam-manager.yaml | 19 ++-----------------
hack/iam-manager_with_webhook.yaml | 3 ++-
6 files changed, 11 insertions(+), 32 deletions(-)
diff --git a/api/v1alpha1/StringOrStrings.go b/api/v1alpha1/StringOrStrings.go
index 412dc76..7110e15 100644
--- a/api/v1alpha1/StringOrStrings.go
+++ b/api/v1alpha1/StringOrStrings.go
@@ -4,7 +4,8 @@ import "encoding/json"
//StringOrStrings type accepts one string or multiple strings
// +kubebuilder:object:generate=false
-// +k8s:openapi-gen=true
+// +kubebuilder:validation:Type=string
+// +kubebuilder:validation:Format=StringOrStrings
type StringOrStrings []string
//MarshalJSON function is a custom implementation of json.Marshal for StringOrStrings
@@ -40,13 +41,3 @@ func (s *StringOrStrings) UnmarshalJSON(b []byte) error {
*s = strings
return nil
}
-
-// OpenAPISchemaType is used by the kube-openapi generator when constructing
-// the OpenAPI spec of this type.
-//
-// See: https://github.com/kubernetes/kube-openapi/tree/master/pkg/generators
-func (StringOrStrings) OpenAPISchemaType() []string { return []string{"string"} }
-
-// OpenAPISchemaFormat is used by the kube-openapi generator when constructing
-// the OpenAPI spec of this type.
-func (StringOrStrings) OpenAPISchemaFormat() string { return "string-or-strings" }
diff --git a/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml b/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml
index 784522a..5696960 100644
--- a/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml
+++ b/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml
@@ -451,9 +451,10 @@ spec:
Principal:
properties:
AWS:
+ format: StringOrStrings
items:
type: string
- type: array
+ type: string
Service:
type: string
type: object
diff --git a/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml b/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml
index 784522a..5696960 100644
--- a/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml
+++ b/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml
@@ -451,9 +451,10 @@ spec:
Principal:
properties:
AWS:
+ format: StringOrStrings
items:
type: string
- type: array
+ type: string
Service:
type: string
type: object
diff --git a/config/crd_no_webhook/kustomization.yaml b/config/crd_no_webhook/kustomization.yaml
index 63d7075..8bfa2d2 100644
--- a/config/crd_no_webhook/kustomization.yaml
+++ b/config/crd_no_webhook/kustomization.yaml
@@ -3,7 +3,7 @@
# It should be run by config/default
resources:
- bases/iammanager.keikoproj.io_iamroles.yaml
-- bases/iammanager.keikoproj.io_iamroles-configmap.yaml
+#- bases/iammanager.keikoproj.io_iamroles-configmap.yaml
# +kubebuilder:scaffold:crdkustomizeresource
patchesStrategicMerge:
diff --git a/hack/iam-manager.yaml b/hack/iam-manager.yaml
index 7985a0b..985c172 100644
--- a/hack/iam-manager.yaml
+++ b/hack/iam-manager.yaml
@@ -459,9 +459,10 @@ spec:
Principal:
properties:
AWS:
+ format: StringOrStrings
items:
type: string
- type: array
+ type: string
Service:
type: string
type: object
@@ -639,22 +640,6 @@ subjects:
namespace: iam-manager-system
---
apiVersion: v1
-data:
- aws.MasterRole: masters.cluster.k8s.local
- aws.accountId: "000011112222"
- iam.managed.permission.boundary.policy: iam-manager-permission-boundary
- iam.managed.policies: shared.cluster.k8s.local
- iam.policy.action.prefix.whitelist: s3:,sts:,ec2:Describe,acm:Describe,acm:List,acm:Get,route53:Get,route53:List,route53:Create,route53:Delete,route53:Change,kms:Decrypt,kms:Encrypt,kms:ReEncrypt,kms:GenerateDataKey,kms:DescribeKey,dynamodb:,secretsmanager:GetSecretValue,es:,sqs:SendMessage,sqs:ReceiveMessage,sqs:DeleteMessage,SNS:Publish,sqs:GetQueueAttributes,sqs:GetQueueUrl
- iam.policy.resource.blacklist: kops
- iam.policy.s3.restricted.resource: '*'
-kind: ConfigMap
-metadata:
- labels:
- app: iam-manager
- name: iam-manager-iamroles-v1alpha1-configmap
- namespace: iam-manager-system
----
-apiVersion: v1
kind: Service
metadata:
annotations:
diff --git a/hack/iam-manager_with_webhook.yaml b/hack/iam-manager_with_webhook.yaml
index 4aaa116..63a1e3b 100644
--- a/hack/iam-manager_with_webhook.yaml
+++ b/hack/iam-manager_with_webhook.yaml
@@ -461,9 +461,10 @@ spec:
Principal:
properties:
AWS:
+ format: StringOrStrings
items:
type: string
- type: array
+ type: string
Service:
type: string
type: object
From 6ea50e17a1b70b006a966f03512cd030b6992df5 Mon Sep 17 00:00:00 2001
From: nmogulla <mnkg561@gmail.com>
Date: Sun, 8 Mar 2020 12:08:11 -0700
Subject: [PATCH 3/4] trust policy changes
---
api/v1alpha1/StringOrStrings.go | 2 --
config/crd/bases/iammanager.keikoproj.io_iamroles.yaml | 3 +--
.../bases/iammanager.keikoproj.io_iamroles.yaml | 3 +--
controllers/iamrole_controller.go | 8 +++++---
go.mod | 2 +-
go.sum | 2 ++
hack/iam-manager.yaml | 3 +--
hack/iam-manager_with_webhook.yaml | 3 +--
pkg/awsapi/iam.go | 2 +-
pkg/awsapi/iam_test.go | 4 ++++
pkg/validation/validate.go | 2 +-
11 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/api/v1alpha1/StringOrStrings.go b/api/v1alpha1/StringOrStrings.go
index 7110e15..0324908 100644
--- a/api/v1alpha1/StringOrStrings.go
+++ b/api/v1alpha1/StringOrStrings.go
@@ -4,8 +4,6 @@ import "encoding/json"
//StringOrStrings type accepts one string or multiple strings
// +kubebuilder:object:generate=false
-// +kubebuilder:validation:Type=string
-// +kubebuilder:validation:Format=StringOrStrings
type StringOrStrings []string
//MarshalJSON function is a custom implementation of json.Marshal for StringOrStrings
diff --git a/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml b/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml
index 5696960..784522a 100644
--- a/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml
+++ b/config/crd/bases/iammanager.keikoproj.io_iamroles.yaml
@@ -451,10 +451,9 @@ spec:
Principal:
properties:
AWS:
- format: StringOrStrings
items:
type: string
- type: string
+ type: array
Service:
type: string
type: object
diff --git a/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml b/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml
index 5696960..784522a 100644
--- a/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml
+++ b/config/crd_no_webhook/bases/iammanager.keikoproj.io_iamroles.yaml
@@ -451,10 +451,9 @@ spec:
Principal:
properties:
AWS:
- format: StringOrStrings
items:
type: string
- type: string
+ type: array
Service:
type: string
type: object
diff --git a/controllers/iamrole_controller.go b/controllers/iamrole_controller.go
index 3f17284..8e5f7e5 100644
--- a/controllers/iamrole_controller.go
+++ b/controllers/iamrole_controller.go
@@ -287,14 +287,16 @@ func (r *IamroleReconciler) UpdateStatus(ctx context.Context, iamRole *iammanage
r.Recorder.Event(iamRole, v1.EventTypeWarning, string(iammanagerv1alpha1.Error), "Unable to create/update status due to error "+err.Error())
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
}
+
+ if state != iammanagerv1alpha1.Error {
+ return ctrl.Result{}, nil
+ }
+
//if wait time is specified, requeue it after provided time
if len(requeueTime) == 0 {
requeueTime[0] = 0
}
- if state != iammanagerv1alpha1.Error {
- return ctrl.Result{}, nil
- }
log.Info("Requeue time", "time", requeueTime[0])
return ctrl.Result{RequeueAfter: time.Duration(requeueTime[0]) * time.Millisecond}, nil
}
diff --git a/go.mod b/go.mod
index 50109c1..2889eab 100644
--- a/go.mod
+++ b/go.mod
@@ -12,7 +12,7 @@ require (
github.com/pkg/errors v0.8.1
github.com/stretchr/testify v1.4.0 // indirect
golang.org/x/sys v0.0.0-20190422165155-953cdadca894 // indirect
- golang.org/x/tools v0.0.0-20200305185322-6a641547f55b // indirect
+ golang.org/x/tools v0.0.0-20200308013534-11ec41452d41 // indirect
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127
k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b
k8s.io/apimachinery v0.0.0-20190404173353-6a84e37a896d
diff --git a/go.sum b/go.sum
index de1d3a4..951658a 100644
--- a/go.sum
+++ b/go.sum
@@ -123,6 +123,8 @@ golang.org/x/tools v0.0.0-20200305140159-d7d444866696 h1:uuiLBSsR+ZDddgZ/2k23Y7F
golang.org/x/tools v0.0.0-20200305140159-d7d444866696/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
golang.org/x/tools v0.0.0-20200305185322-6a641547f55b h1:kWwtroURwYKTlrhKtLws/aJ3iuNdVB417e2FCSutkIs=
golang.org/x/tools v0.0.0-20200305185322-6a641547f55b/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
+golang.org/x/tools v0.0.0-20200308013534-11ec41452d41 h1:9Di9iYgOt9ThCipBxChBVhgNipDoE5mxO84rQV7D0FE=
+golang.org/x/tools v0.0.0-20200308013534-11ec41452d41/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/hack/iam-manager.yaml b/hack/iam-manager.yaml
index 985c172..a7cc98e 100644
--- a/hack/iam-manager.yaml
+++ b/hack/iam-manager.yaml
@@ -459,10 +459,9 @@ spec:
Principal:
properties:
AWS:
- format: StringOrStrings
items:
type: string
- type: string
+ type: array
Service:
type: string
type: object
diff --git a/hack/iam-manager_with_webhook.yaml b/hack/iam-manager_with_webhook.yaml
index 63a1e3b..4aaa116 100644
--- a/hack/iam-manager_with_webhook.yaml
+++ b/hack/iam-manager_with_webhook.yaml
@@ -461,10 +461,9 @@ spec:
Principal:
properties:
AWS:
- format: StringOrStrings
items:
type: string
- type: string
+ type: array
Service:
type: string
type: object
diff --git a/pkg/awsapi/iam.go b/pkg/awsapi/iam.go
index e67e2a0..b45668f 100644
--- a/pkg/awsapi/iam.go
+++ b/pkg/awsapi/iam.go
@@ -129,7 +129,7 @@ func (i *IAM) CreateRole(ctx context.Context, req IAMRoleRequest) (*IAMRoleRespo
}
log.V(1).Info("Attaching Inline role policies")
- return i.AttachInlineRolePolicy(ctx, req)
+ return i.UpdateRole(ctx, req)
}
//TagRole tags role with appropriate tags
diff --git a/pkg/awsapi/iam_test.go b/pkg/awsapi/iam_test.go
index 903d633..ea52f30 100644
--- a/pkg/awsapi/iam_test.go
+++ b/pkg/awsapi/iam_test.go
@@ -55,6 +55,8 @@ func (s *IAMAPISuite) TestCreateRoleSuccess(c *check.C) {
s.mockI.EXPECT().PutRolePermissionsBoundary(&iam.PutRolePermissionsBoundaryInput{RoleName: aws.String("VALID_ROLE"), PermissionsBoundary: aws.String(config.Props.ManagedPermissionBoundaryPolicy())}).Times(1).Return(nil, nil)
s.mockI.EXPECT().PutRolePolicy(&iam.PutRolePolicyInput{PolicyDocument: aws.String("SOMETHING"), RoleName: aws.String("VALID_ROLE"), PolicyName: aws.String("VALID_POLICY")}).Times(1).Return(&iam.PutRolePolicyOutput{}, nil)
s.mockI.EXPECT().AttachRolePolicy(&iam.AttachRolePolicyInput{PolicyArn: aws.String("arn:aws:iam::123456789012:policy/SOMETHING"), RoleName: aws.String("VALID_ROLE")}).Times(1).Return(&iam.AttachRolePolicyOutput{}, nil)
+ s.mockI.EXPECT().UpdateRole(&iam.UpdateRoleInput{RoleName: aws.String("VALID_ROLE"), MaxSessionDuration: aws.Int64(3600), Description: aws.String("")}).Times(1).Return(&iam.UpdateRoleOutput{}, nil)
+ s.mockI.EXPECT().UpdateAssumeRolePolicy(&iam.UpdateAssumeRolePolicyInput{RoleName: aws.String("VALID_ROLE"), PolicyDocument: aws.String("SOMETHING")}).Times(1).Return(&iam.UpdateAssumeRolePolicyOutput{}, nil)
req := awsapi.IAMRoleRequest{Name: "VALID_ROLE", PolicyName: "VALID_POLICY", PermissionPolicy: "SOMETHING", SessionDuration: 3600, TrustPolicy: "SOMETHING", ManagedPermissionBoundaryPolicy: config.Props.ManagedPermissionBoundaryPolicy(), ManagedPolicies: config.Props.ManagedPolicies()}
_, err := s.mockIAM.CreateRole(s.ctx, req)
c.Assert(err, check.IsNil)
@@ -71,6 +73,8 @@ func (s *IAMAPISuite) TestCreateRoleSuccessWithUpdate(c *check.C) {
s.mockI.EXPECT().PutRolePermissionsBoundary(&iam.PutRolePermissionsBoundaryInput{RoleName: aws.String("VALID_ROLE"), PermissionsBoundary: aws.String(config.Props.ManagedPermissionBoundaryPolicy())}).Times(1).Return(nil, nil)
s.mockI.EXPECT().PutRolePolicy(&iam.PutRolePolicyInput{PolicyDocument: aws.String("SOMETHING"), RoleName: aws.String("VALID_ROLE"), PolicyName: aws.String("VALID_POLICY")}).Times(1).Return(&iam.PutRolePolicyOutput{}, nil)
s.mockI.EXPECT().AttachRolePolicy(&iam.AttachRolePolicyInput{PolicyArn: aws.String("arn:aws:iam::123456789012:policy/SOMETHING"), RoleName: aws.String("VALID_ROLE")}).Times(1).Return(&iam.AttachRolePolicyOutput{}, nil)
+ s.mockI.EXPECT().UpdateRole(&iam.UpdateRoleInput{RoleName: aws.String("VALID_ROLE"), MaxSessionDuration: aws.Int64(3600), Description: aws.String("")}).Times(1).Return(&iam.UpdateRoleOutput{}, nil)
+ s.mockI.EXPECT().UpdateAssumeRolePolicy(&iam.UpdateAssumeRolePolicyInput{RoleName: aws.String("VALID_ROLE"), PolicyDocument: aws.String("SOMETHING")}).Times(1).Return(&iam.UpdateAssumeRolePolicyOutput{}, nil)
req := awsapi.IAMRoleRequest{Name: "VALID_ROLE", PolicyName: "VALID_POLICY", PermissionPolicy: "SOMETHING", SessionDuration: 3600, TrustPolicy: "SOMETHING", ManagedPermissionBoundaryPolicy: config.Props.ManagedPermissionBoundaryPolicy(), ManagedPolicies: config.Props.ManagedPolicies()}
_, err := s.mockIAM.CreateRole(s.ctx, req)
c.Assert(err, check.IsNil)
diff --git a/pkg/validation/validate.go b/pkg/validation/validate.go
index 78dfdba..9465718 100644
--- a/pkg/validation/validate.go
+++ b/pkg/validation/validate.go
@@ -154,7 +154,7 @@ func CompareAssumeRolePolicy(ctx context.Context, request awsapi.IAMRoleRequest,
}
//compare
if !reflect.DeepEqual(reqAssume, destAssume) {
- log.Info("input assume role policy and target assume role policy are NOT equal")
+ log.Info("input assume role policy and target assume role policy are NOT equal", "req", reqAssume, "dest", destAssume)
return false
}
From 92b97261a4d15198a3a3231d3b498a9c41ea75b0 Mon Sep 17 00:00:00 2001
From: nmogulla <mnkg561@gmail.com>
Date: Mon, 9 Mar 2020 15:53:13 -0700
Subject: [PATCH 4/4] Taking care of review comments
---
go.mod | 3 ++-
go.sum | 18 ------------------
internal/config/properties_test.go | 2 +-
pkg/awsapi/iam.go | 2 +-
pkg/validation/validate.go | 20 ++++++++++----------
pkg/validation/validate_test.go | 12 ++++++------
6 files changed, 20 insertions(+), 37 deletions(-)
diff --git a/go.mod b/go.mod
index 2889eab..dfbde5c 100644
--- a/go.mod
+++ b/go.mod
@@ -11,8 +11,9 @@ require (
github.com/pborman/uuid v0.0.0-20170612153648-e790cca94e6c
github.com/pkg/errors v0.8.1
github.com/stretchr/testify v1.4.0 // indirect
+ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 // indirect
+ golang.org/x/net v0.0.0-20200226121028-0de0cce0169b // indirect
golang.org/x/sys v0.0.0-20190422165155-953cdadca894 // indirect
- golang.org/x/tools v0.0.0-20200308013534-11ec41452d41 // indirect
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127
k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b
k8s.io/apimachinery v0.0.0-20190404173353-6a84e37a896d
diff --git a/go.sum b/go.sum
index 951658a..8216270 100644
--- a/go.sum
+++ b/go.sum
@@ -20,8 +20,6 @@ github.com/gogo/protobuf v1.1.1 h1:72R+M5VuhED/KujmZVcIquuo8mBgX4oVda//DQb3PXo=
github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
github.com/golang/groupcache v0.0.0-20180513044358-24b0969c4cb7 h1:u4bArs140e9+AfE52mFHOXVFnOSBJBRlzTHrOPLOIhE=
github.com/golang/groupcache v0.0.0-20180513044358-24b0969c4cb7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
-github.com/golang/mock v1.4.0 h1:Rd1kQnQu0Hq3qvJppYSG0HtP+f5LPPUiDswTLiEegLg=
-github.com/golang/mock v1.4.0/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/mock v1.4.1 h1:ocYkMQY5RrXTYgXl7ICpV0IXwlEQGwKIsery4gyXa1U=
github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
@@ -89,20 +87,16 @@ golang.org/x/crypto v0.0.0-20180820150726-614d502a4dac/go.mod h1:6SG95UA2DQfeDnf
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 h1:ObdrDkeb4kJdCP557AjRjq69pTHfNouLtWZG7j9rPN8=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
-golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/net v0.0.0-20180906233101-161cd47e91fd h1:nTDtHvHSdCn1m6ITfMRqtOd/9+7a3s8RBNOZ3eYZzJA=
golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
-golang.org/x/net v0.0.0-20190620200207-3b0461eec859 h1:R/3boaszxrf1GEUWTVDzSKVwLmSJpwZ1yqXm8j0v2QI=
-golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20200226121028-0de0cce0169b h1:0mm1VjtFUOIlE1SbDlwjYaDxZVDP2S5ou6y0gSgXHu8=
golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be h1:vEDujvNQGv4jgYKudGeI/+DAX4Jffq6hpD55MmoEvKs=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e h1:o3PsSEY8E4eXWkXrIP9YJALUkVZqzHJT5DOasTyn8Vs=
golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
@@ -116,18 +110,6 @@ golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 h1:+DCIGbF/swA92ohVg0//6X2I
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/tools v0.0.0-20190425150028-36563e24a262 h1:qsl9y/CJx34tuA7QCPNp86JNJe4spst6Ff8MjvPUdPg=
golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
-golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
-golang.org/x/tools v0.0.0-20200304193943-95d2e580d8eb h1:iKlO7ROJc6SttHKlxzwGytRtBUqX4VARrNTgP2YLX5M=
-golang.org/x/tools v0.0.0-20200304193943-95d2e580d8eb/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
-golang.org/x/tools v0.0.0-20200305140159-d7d444866696 h1:uuiLBSsR+ZDddgZ/2k23Y7FrUNl29gq4sEFcO170R5k=
-golang.org/x/tools v0.0.0-20200305140159-d7d444866696/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
-golang.org/x/tools v0.0.0-20200305185322-6a641547f55b h1:kWwtroURwYKTlrhKtLws/aJ3iuNdVB417e2FCSutkIs=
-golang.org/x/tools v0.0.0-20200305185322-6a641547f55b/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
-golang.org/x/tools v0.0.0-20200308013534-11ec41452d41 h1:9Di9iYgOt9ThCipBxChBVhgNipDoE5mxO84rQV7D0FE=
-golang.org/x/tools v0.0.0-20200308013534-11ec41452d41/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
-golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
-golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
-golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gomodules.xyz/jsonpatch/v2 v2.0.1 h1:xyiBuvkD2g5n7cYzx6u2sxQvsAy4QJsZFCzGVdzOXZ0=
gomodules.xyz/jsonpatch/v2 v2.0.1/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU=
google.golang.org/appengine v1.1.0 h1:igQkv0AAhEIvTEpD5LIpAfav2eeVO9HBTjvKHVJPRSs=
diff --git a/internal/config/properties_test.go b/internal/config/properties_test.go
index 0671262..bbbde72 100644
--- a/internal/config/properties_test.go
+++ b/internal/config/properties_test.go
@@ -121,7 +121,7 @@ func (s *PropertiesSuite) TestGetAWSRegion(c *check.C) {
c.Assert(value, check.NotNil)
}
-func (s *PropertiesSuite) TestGetAWSMasterRole(c *check.C) {
+func (s *PropertiesSuite) TestTrustPolicyARNs(c *check.C) {
value := Props.TrustPolicyARNs()
c.Assert(value, check.NotNil)
}
diff --git a/pkg/awsapi/iam.go b/pkg/awsapi/iam.go
index b45668f..c974400 100644
--- a/pkg/awsapi/iam.go
+++ b/pkg/awsapi/iam.go
@@ -335,7 +335,7 @@ func (i *IAM) AttachInlineRolePolicy(ctx context.Context, req IAMRoleRequest) (*
return &IAMRoleResponse{}, nil
}
-//GetRolePolicy gets the role from aws iam
+//GetRole gets the role from aws iam
func (i *IAM) GetRole(ctx context.Context, req IAMRoleRequest) (*iam.GetRoleOutput, error) {
log := log.Logger(ctx, "awsapi", "iam", "GetRole")
log.WithValues("roleName", req.Name)
diff --git a/pkg/validation/validate.go b/pkg/validation/validate.go
index 9465718..cd5e414 100644
--- a/pkg/validation/validate.go
+++ b/pkg/validation/validate.go
@@ -90,20 +90,20 @@ func ValidateIAMPolicyResource(ctx context.Context, pDoc v1alpha1.PolicyDocument
}
//CompareRole function compares input role to target role
-func CompareRole(ctx context.Context, request awsapi.IAMRoleRequest, role *iam.GetRoleOutput, target string) bool {
+func CompareRole(ctx context.Context, request awsapi.IAMRoleRequest, targetRole *iam.GetRoleOutput, targetRolePolicy string) bool {
log := log.Logger(ctx, "pkg.validation", "ComparePolicy")
// Step 1: Compare the permission policy
- if !ComparePermissionPolicy(ctx, request, target) {
+ if !ComparePermissionPolicy(ctx, request.PermissionPolicy, targetRolePolicy) {
return false
}
//Step 2: Compare Assume Role Policy Document
- if !CompareAssumeRolePolicy(ctx, request, role) {
+ if !CompareAssumeRolePolicy(ctx, request.TrustPolicy, *targetRole.Role.AssumeRolePolicyDocument) {
return false
}
//Step 3: Compare Permission Boundary
- if !reflect.DeepEqual(request.ManagedPermissionBoundaryPolicy, *role.Role.PermissionsBoundary.PermissionsBoundaryArn) {
+ if !reflect.DeepEqual(request.ManagedPermissionBoundaryPolicy, *targetRole.Role.PermissionsBoundary.PermissionsBoundaryArn) {
log.Info("input permission boundary and target permission boundary are NOT equal")
return false
}
@@ -111,8 +111,8 @@ func CompareRole(ctx context.Context, request awsapi.IAMRoleRequest, role *iam.G
return true
}
-//ComparePermissionPolicy compares assume role policy from request and response
-func ComparePermissionPolicy(ctx context.Context, request awsapi.IAMRoleRequest, target string) bool {
+//ComparePermissionPolicy compares role policy from request and response
+func ComparePermissionPolicy(ctx context.Context, request string, target string) bool {
log := log.Logger(ctx, "pkg.validation", "CompareAssumeRolePolicy")
d, _ := url.QueryUnescape(target)
@@ -123,7 +123,7 @@ func ComparePermissionPolicy(ctx context.Context, request awsapi.IAMRoleRequest,
}
req := v1alpha1.PolicyDocument{}
- err = json.Unmarshal([]byte(request.PermissionPolicy), &req)
+ err = json.Unmarshal([]byte(request), &req)
if err != nil {
log.Error(err, "failed to marshal policy document")
}
@@ -137,10 +137,10 @@ func ComparePermissionPolicy(ctx context.Context, request awsapi.IAMRoleRequest,
}
//CompareAssumeRolePolicy compares assume role policy from request and response
-func CompareAssumeRolePolicy(ctx context.Context, request awsapi.IAMRoleRequest, role *iam.GetRoleOutput) bool {
+func CompareAssumeRolePolicy(ctx context.Context, request string, target string) bool {
log := log.Logger(ctx, "pkg.validation", "CompareAssumeRolePolicy")
- a, _ := url.QueryUnescape(*role.Role.AssumeRolePolicyDocument)
+ a, _ := url.QueryUnescape(target)
destAssume := utils.TrustPolicy{}
err := json.Unmarshal([]byte(a), &destAssume)
if err != nil {
@@ -148,7 +148,7 @@ func CompareAssumeRolePolicy(ctx context.Context, request awsapi.IAMRoleRequest,
}
reqAssume := utils.TrustPolicy{}
- err = json.Unmarshal([]byte(request.TrustPolicy), &reqAssume)
+ err = json.Unmarshal([]byte(request), &reqAssume)
if err != nil {
log.Error(err, "failed to marshal assume role policy document")
}
diff --git a/pkg/validation/validate_test.go b/pkg/validation/validate_test.go
index 37ab365..7df79a3 100644
--- a/pkg/validation/validate_test.go
+++ b/pkg/validation/validate_test.go
@@ -190,7 +190,7 @@ func (s *ValidateSuite) TestComparePermissionPolicySuccess(c *check.C) {
PermissionPolicy: string(role1),
}
- flag := validation.ComparePermissionPolicy(s.ctx, i1, string(role2))
+ flag := validation.ComparePermissionPolicy(s.ctx, i1.PermissionPolicy, string(role2))
c.Assert(flag, check.Equals, true)
}
@@ -220,7 +220,7 @@ func (s *ValidateSuite) TestComparePermissionPolicy2Success(c *check.C) {
PermissionPolicy: string(role1),
}
- flag := validation.ComparePermissionPolicy(s.ctx, i1, string(role2))
+ flag := validation.ComparePermissionPolicy(s.ctx, i1.PermissionPolicy, string(role2))
c.Assert(flag, check.Equals, true)
}
@@ -250,7 +250,7 @@ func (s *ValidateSuite) TestComparePermissionPolicyFailure(c *check.C) {
PermissionPolicy: string(role1),
}
- flag := validation.ComparePermissionPolicy(s.ctx, i1, string(role2))
+ flag := validation.ComparePermissionPolicy(s.ctx, i1.PermissionPolicy, string(role2))
c.Assert(flag, check.Equals, false)
}
@@ -294,7 +294,7 @@ func (s *ValidateSuite) TestCompareAssumeRolePolicySuccess(c *check.C) {
TrustPolicy: string(role1),
}
- flag := validation.CompareAssumeRolePolicy(s.ctx, i1, &target)
+ flag := validation.CompareAssumeRolePolicy(s.ctx, i1.TrustPolicy, *target.Role.AssumeRolePolicyDocument)
c.Assert(flag, check.Equals, true)
}
@@ -337,7 +337,7 @@ func (s *ValidateSuite) TestCompareAssumeRolePolicy2Success(c *check.C) {
TrustPolicy: string(role1),
}
- flag := validation.CompareAssumeRolePolicy(s.ctx, i1, &target)
+ flag := validation.CompareAssumeRolePolicy(s.ctx, i1.TrustPolicy, *target.Role.AssumeRolePolicyDocument)
c.Assert(flag, check.Equals, true)
}
@@ -380,7 +380,7 @@ func (s *ValidateSuite) TestCompareAssumeRolePolicyFailure(c *check.C) {
TrustPolicy: string(role1),
}
- flag := validation.CompareAssumeRolePolicy(s.ctx, i1, &target)
+ flag := validation.CompareAssumeRolePolicy(s.ctx, i1.TrustPolicy, *target.Role.AssumeRolePolicyDocument)
c.Assert(flag, check.Equals, false)
}