Merge pull request #43 from mnkg561/bug_trust_role

Bug trust role

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:

api/v1alpha1/StringOrStrings.go

@@ -0,0 +1,41 @@
+package v1alpha1
+
+import "encoding/json"
+
+//StringOrStrings type accepts one string or multiple strings
+// +kubebuilder:object:generate=false
+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
+}

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"

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
+}

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

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

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:

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,32 +116,25 @@ 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")
 	log.WithValues("iamrole", iamRole.Name)
 	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
 		}
@@ -282,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
 }

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 |
+| aws.region                        | AWS Region                    | us-west-2          | Required |
+| iam.default.trust.policy.role.arn.list| Default trust policy role arn list |           | Optional |

go.mod

@@ -5,14 +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/net v0.0.0-20200226121028-0de0cce0169b // indirect
 	golang.org/x/sys v0.0.0-20190422165155-953cdadca894 // indirect
 	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127
 	k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b

go.sum

@@ -20,8 +20,8 @@ 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=
 github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs=
 github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
@@ -91,8 +91,8 @@ golang.org/x/net v0.0.0-20180906233101-161cd47e91fd h1:nTDtHvHSdCn1m6ITfMRqtOd/9
 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=