From f62212a5b462b922d4264397b0403931580b3fc0 Mon Sep 17 00:00:00 2001 From: Lun-Kai Hsu Date: Tue, 18 Sep 2018 15:11:02 -0700 Subject: [PATCH] Webapp: Don't use DM for IAM. (#1550) * fix * review * review --- bootstrap/cmd/bootstrap/app/gcpUtils.go | 126 ++++++++++++- bootstrap/cmd/bootstrap/app/ksServer.go | 39 ++++ .../src/configs/cluster.jinja | 170 ------------------ .../src/configs/iam_bindings_template.yaml | 41 +++++ 4 files changed, 205 insertions(+), 171 deletions(-) create mode 100644 components/gcp-click-to-deploy/src/configs/iam_bindings_template.yaml diff --git a/bootstrap/cmd/bootstrap/app/gcpUtils.go b/bootstrap/cmd/bootstrap/app/gcpUtils.go index 2feda2fc5a2..32ea19eead3 100644 --- a/bootstrap/cmd/bootstrap/app/gcpUtils.go +++ b/bootstrap/cmd/bootstrap/app/gcpUtils.go @@ -1,10 +1,15 @@ package app import ( + "fmt" + "time" + "golang.org/x/net/context" "google.golang.org/api/deploymentmanager/v2" "golang.org/x/oauth2" "github.com/ghodss/yaml" + log "github.com/sirupsen/logrus" + "google.golang.org/api/cloudresourcemanager/v1" "io/ioutil" "path" ) @@ -20,6 +25,23 @@ type DmConf struct { Resources []Resource `json:"resources"` } +type IamBinding struct { + Members []string `type:"members` + Roles []string `type:"roles"` +} + +type IamConf struct { + IamBindings []IamBinding `json:"bindings"` +} + +type ApplyIamRequest struct { + Project string `json:"project"` + Cluster string `json:"cluster"` + Email string `json:"email"` + Token string `json:"token"` + Action string `json:"action` +} + // TODO: handle concurrent & repetitive deployment requests. func (s *ksServer)InsertDeployment(ctx context.Context, req CreateRequest) error { regPath := s.knownRegistries["kubeflow"].RegUri @@ -83,4 +105,106 @@ func (s *ksServer)GetDeploymentStatus(ctx context.Context, req CreateRequest) (s return "", err } return dm.Operation.Status, nil -} \ No newline at end of file +} + +func GetUpdatedPolicy(currentPolicy *cloudresourcemanager.Policy, iamConf *IamConf, req ApplyIamRequest) cloudresourcemanager.Policy { + // map from role to members. + policyMap := map[string]map[string]bool {} + for _, binding := range currentPolicy.Bindings { + policyMap[binding.Role] = make(map[string]bool) + for _, member := range binding.Members { + policyMap[binding.Role][member] = true + } + } + + // Replace placeholder with actual identity. + saMapping := map[string]string { + "set-kubeflow-admin-service-account": fmt.Sprintf("serviceAccount:%v-admin@%v.iam.gserviceaccount.com", req.Cluster, req.Project), + "set-kubeflow-user-service-account": fmt.Sprintf("serviceAccount:%v-user@%v.iam.gserviceaccount.com", req.Cluster, req.Project), + "set-kubeflow-vm-service-account": fmt.Sprintf("serviceAccount:%v-vm@%v.iam.gserviceaccount.com", req.Cluster, req.Project), + "set-kubeflow-iap-account": fmt.Sprintf("user:%v", req.Email), + } + for _, binding := range iamConf.IamBindings { + for _, member := range binding.Members { + actualMember := member + if val, ok := saMapping[member]; ok { + actualMember = val + } + for _, role := range binding.Roles { + if req.Action == "add" { + policyMap[role][actualMember] = true + } else { + // action == "remove" + policyMap[role][actualMember] = false + } + } + } + } + newPolicy := cloudresourcemanager.Policy{} + for role, memberSet := range policyMap { + binding := cloudresourcemanager.Binding{} + binding.Role = role + for member, exists := range memberSet { + if exists { + binding.Members = append(binding.Members, member) + } + } + newPolicy.Bindings = append(newPolicy.Bindings, &binding) + } + return newPolicy +} + +func (s *ksServer)ApplyIamPolicy(ctx context.Context, req ApplyIamRequest) error { + // Get the iam change from config. + regPath := s.knownRegistries["kubeflow"].RegUri + templatePath := path.Join(regPath, "../components/gcp-click-to-deploy/src/configs/iam_bindings_template.yaml") + var iamConf IamConf + err := LoadConfig(templatePath, &iamConf) + if err != nil { + log.Errorf("Failed to load iam config: %v", err) + return err + } + + ts := oauth2.StaticTokenSource(&oauth2.Token{ + AccessToken: req.Token, + }) + resourceManager, err := cloudresourcemanager.New(oauth2.NewClient(ctx, ts)) + if err != nil { + log.Errorf("Cannot create resource manager client: %v", err) + return err + } + s.serverMux.Lock() + defer s.serverMux.Unlock() + + retry := 0 + for retry < 5 { + // Get current policy + saPolicy, err := resourceManager.Projects.GetIamPolicy( + req.Project, + &cloudresourcemanager.GetIamPolicyRequest{ + }).Do() + if err != nil { + retry += 1 + log.Warningf("Cannot get current policy: %v", err) + time.Sleep(3 * time.Second) + continue + } + + // Get the updated policy and apply it. + newPolicy := GetUpdatedPolicy(saPolicy, &iamConf, req) + _, err = resourceManager.Projects.SetIamPolicy( + req.Project, + &cloudresourcemanager.SetIamPolicyRequest{ + Policy: &newPolicy, + }).Do() + if err != nil { + retry += 1 + log.Warningf("Cannot set new ploicy: %v", err) + time.Sleep(3 * time.Second) + } + } + if err != nil { + return err + } + return nil +} diff --git a/bootstrap/cmd/bootstrap/app/ksServer.go b/bootstrap/cmd/bootstrap/app/ksServer.go index 81372879be3..12865dafc05 100644 --- a/bootstrap/cmd/bootstrap/app/ksServer.go +++ b/bootstrap/cmd/bootstrap/app/ksServer.go @@ -48,6 +48,7 @@ type KsService interface { BindRole(context.Context, string, string, string) error InsertDeployment(context.Context, CreateRequest) error GetDeploymentStatus(context.Context, CreateRequest) (string, error) + ApplyIamPolicy(context.Context, ApplyIamRequest) error } // appInfo keeps track of information about apps. @@ -813,6 +814,19 @@ func finishDeployment(svc KsService, req CreateRequest) { return } + log.Info("Patching IAM bindings...") + err = svc.ApplyIamPolicy(ctx, ApplyIamRequest{ + Project: req.Project, + Cluster: req.Cluster, + Email: req.Email, + Token: req.Token, + Action: "add", + }) + if err != nil { + log.Errorf("Failed to update IAM: %v", err) + return + } + log.Infof("Inserting sa keys...") err = svc.InsertSaKeys(ctx, InsertSaKeyRequest{ Cluster: req.Cluster, @@ -897,6 +911,18 @@ func makeSaKeyEndpoint(svc KsService) endpoint.Endpoint { } } +func makeIamEndpoint(svc KsService) endpoint.Endpoint { + return func(ctx context.Context, request interface{}) (interface{}, error) { + req := request.(ApplyIamRequest) + err := svc.ApplyIamPolicy(ctx, req) + r := &basicServerResponse{} + if err != nil { + r.Err = err.Error() + } + return r, nil + } +} + func decodeCreateAppRequest(_ context.Context, r *http.Request) (interface{}, error) { var request CreateRequest if err := json.NewDecoder(r.Body).Decode(&request); err != nil { @@ -971,6 +997,18 @@ func (s *ksServer) StartHttp(port int) { encodeResponse, ) + applyIamHandler := httptransport.NewServer( + makeIamEndpoint(s), + func (_ context.Context, r *http.Request) (interface{}, error) { + var request ApplyIamRequest + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + return nil, err + } + return request, nil + }, + encodeResponse, + ) + initProjectHandler := httptransport.NewServer( makeInitProjectEndpoint(s), func (_ context.Context, r *http.Request) (interface{}, error) { @@ -995,6 +1033,7 @@ func (s *ksServer) StartHttp(port int) { http.Handle("/kfctl/apps/apply", optionsHandler(applyAppHandler)) http.Handle("/kfctl/apps/create", optionsHandler(createAppHandler)) http.Handle("/kfctl/iam/insertSaKey", optionsHandler(insertSaKeyHandler)) + http.Handle("/kfctl/iam/apply", optionsHandler(applyIamHandler)) http.Handle("/kfctl/initProject", optionsHandler(initProjectHandler)) http.Handle("/kfctl/e2eDeploy", optionsHandler(deployHandler)) diff --git a/components/gcp-click-to-deploy/src/configs/cluster.jinja b/components/gcp-click-to-deploy/src/configs/cluster.jinja index b9afd4897df..86829e256e9 100644 --- a/components/gcp-click-to-deploy/src/configs/cluster.jinja +++ b/components/gcp-click-to-deploy/src/configs/cluster.jinja @@ -162,176 +162,6 @@ resources: properties: description: "Static IP for Kubeflow ingress." - -{# Get the IAM policy first so that we do not remove any existing bindings. #} -- name: get-iam-policy-add - action: gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.getIamPolicy - properties: - resource: {{ env["project"] }} -{# Set the IAM policy patching the existing policy with what ever is currently in the - config. - - We need to make the cloudservices account a GKE cluster admin because deployment manager - users the cloudservices account; so this will be the identity used with the K*s cluster. - - Note: This will fail if the cloudservices account doesn't have IamProjectAdmin - permissions. -#} -- name: set-iam-policy-add - action: gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.setIamPolicy - metadata: - runtimePolicy: - - CREATE - properties: - resource: {{ env["project"] }} - policy: $(ref.get-iam-policy-add) - gcpIamPolicyPatch: - add: - - role: roles/container.admin - members: - {# Deployment manager uses cloudservices account. #} - - {{ 'serviceAccount:' + env['project_number'] + '@cloudservices.gserviceaccount.com' }} - {# Grant permissions needed to submit builds to Google Cloud Container Builder #} - - role: roles/cloudbuild.builds.editor - members: - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - {# roles/viewer is required for viewing the logs of a GCB build #} - - role: roles/viewer - members: - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - {# Grant permissions needed to push the app to a cloud repository. #} - - role: roles/source.admin - members: - - {{ 'serviceAccount:' + KF_ADMIN_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - {# servicemanagement.admin is needed by CloudEndpoints controller - so we can create a service to get a hostname. - #} - - role: roles/servicemanagement.admin - members: - - {{ 'serviceAccount:' + KF_ADMIN_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - {# Network admin is needed to enable IAP and configure network settings - like backend timeouts and health checks. - #} - - role: roles/compute.networkAdmin - members: - - {{ 'serviceAccount:' + KF_ADMIN_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/storage.admin - members: - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/bigquery.admin - members: - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/dataflow.admin - members: - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/logging.logWriter - members: - {# VM service account is used to write logs. #} - - {{ 'serviceAccount:' + KF_VM_SA_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/monitoring.metricWriter - members: - {# VM service account is used to write monitoring data. #} - - {{ 'serviceAccount:' + KF_VM_SA_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/storage.objectViewer - members: - {# VM service account is used to pull image from gcr. #} - - {{ 'serviceAccount:' + KF_VM_SA_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - {% if 'users' in properties %} - {% if properties['users'] %} - - role: roles/iap.httpsResourceAccessor - members: - {% for user in properties['users'] %} - - {{ user }} - {% endfor %} - {% endif %} - {% endif %} -- name: set-iam-policy-delete - action: gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.setIamPolicy - metadata: - runtimePolicy: - - DELETE - properties: - resource: {{ env["project"] }} - policy: $(ref.set-iam-policy-add) - gcpIamPolicyPatch: - remove: - {# Grant permissions needed to submit builds to Google Cloud Container Builder #} - - role: roles/cloudbuild.builds.editor - members: - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - {# roles/viewer is required for viewing the logs of a GCB build #} - - role: roles/viewer - members: - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - {# Grant permissions needed to push the app to a cloud repository. #} - - role: roles/source.admin - members: - - {{ 'serviceAccount:' + KF_ADMIN_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - {# servicemanagement.admin is needed by CloudEndpoints controller - so we can create a service to get a hostname. - #} - - role: roles/servicemanagement.admin - members: - - {{ 'serviceAccount:' + KF_ADMIN_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - {# Network admin is needed to enable IAP and configure network settings - like backend timeouts and health checks. - #} - - role: roles/compute.networkAdmin - members: - - {{ 'serviceAccount:' + KF_ADMIN_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/storage.admin - members: - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/bigquery.admin - members: - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/dataflow.admin - members: - - {{ 'serviceAccount:' + KF_USER_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/logging.logWriter - members: - {# VM service account is used to write logs. #} - - {{ 'serviceAccount:' + KF_VM_SA_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/monitoring.metricWriter - members: - {# VM service account is used to write monitoring data. #} - - {{ 'serviceAccount:' + KF_VM_SA_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - - role: roles/storage.objectViewer - members: - {# VM service account is used to pull image from gcr. #} - - {{ 'serviceAccount:' + KF_VM_SA_NAME + '@' + env['project'] + '.iam.gserviceaccount.com' }} - - {% if 'users' in properties %} - {% if properties['users'] %} - - role: roles/iap.httpsResourceAccessor - members: - {% for user in properties['users'] %} - - {{ user }} - {% endfor %} - {% endif %} - {% endif %} - {# This changes every time to ensure a fresh etag is obtained. #} - quotaUser: {{ env["current_time"] }} - {# Define TypeProviders for different K8s endpoints. https://cloud.google.com/deployment-manager/docs/configuration/type-providers/process-adding-api diff --git a/components/gcp-click-to-deploy/src/configs/iam_bindings_template.yaml b/components/gcp-click-to-deploy/src/configs/iam_bindings_template.yaml new file mode 100644 index 00000000000..752c5dcd73d --- /dev/null +++ b/components/gcp-click-to-deploy/src/configs/iam_bindings_template.yaml @@ -0,0 +1,41 @@ +# This config is used by go binary. It is not a DM config. +# +# Schema for this yaml file +# * bindings is a list of (members, roles) dict +# * members and roles are lists +# * each role in roles is granted to each member in members +bindings: +- members: + - set-kubeflow-admin-service-account + roles: + # Grant permissions needed to push the app to a cloud repository + - roles/source.admin + # servicemanagement.admin is needed by CloudEndpoints controller so we can create a service to get a hostname. + - roles/servicemanagement.admin + # Network admin is needed to enable IAP and configure network settings like backend timeouts and health checks + - roles/compute.networkAdmin +- members: + - set-kubeflow-user-service-account + roles: + # Grant permissions needed to submit builds to Google Cloud Container Builder + - roles/cloudbuild.builds.editor + # roles/viewer is required for viewing the logs of a GCB build + - roles/viewer + # Grant permissions needed to push the app to a cloud repository + - roles/source.admin + - roles/storage.admin + - roles/bigquery.admin + - roles/dataflow.admin +- members: + - set-kubeflow-vm-service-account + roles: + # VM service account is used to write logs + - roles/logging.logWriter + # VM service account is used to write monitoring data + - roles/monitoring.metricWriter + # VM service account is used to pull image from gcr + - roles/storage.objectViewer +- members: + - set-kubeflow-iap-account + roles: + - roles/iap.httpsResourceAccessor \ No newline at end of file