Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend identity documentation to GCP #29

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

slash-under
Copy link
Member

No description provided.

@skuenzli skuenzli self-requested a review April 7, 2020 23:29
…ngineering to 'Helpful', from 'he'. Removed irrelevant links and updated design with enforced name of the GCP Org.

The DevOps team will provision two 'delivery' accounts for each project team: `dev` and `prod`. Teams will use the
`dev` account to develop their applications and test application deployments. Applications should be delivered
to the production account for operation and use by customers and end users.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qq: should each HE project get separate dev and prod accounts in GCP? what advantages does this have over a dev and prod GCP project within a single GCP account for the HE project?

I definitely think we should create strong isolation between environments and projects, just wondering what the right balance is. AFAIK, a GCP IAM principal:

  • must be specifically granted access to resources in different projects
  • may be granted permissions to call different API actions in different projects in a single account (is this true?)

The DevOps team will use [GCP Cloud Identity](https://cloud.google.com/identity/docs)
to provision both the shared and project accounts. Control Tower provides a simple account provisioning model that
provides a number of security and governance best practices out of the box.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to update/replace references to Service Control Policy and Control Tower.

Copy link
Contributor

@skuenzli skuenzli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested several updates and a question about accounts vs projects.

Copy link
Contributor

@FreemanParks FreemanParks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very minor changes requested. Everything else looks great!

### UC2 - Provision Accounts

The HelpfulEng DevOps team will provision GCP accounts for both shared and project delivery accounts. The DevOps team
would like provision these accounts in a standardized way with low effort and simple adoption of Cloud security and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor style/formatting

Suggested change
would like provision these accounts in a standardized way with low effort and simple adoption of Cloud security and
would like to provision these accounts in a standardized way with low effort and simple adoption of Cloud security and

* A trusted external repository such as Docker Hub.
* An internal repository such as a [Cloud Storage](https://cloud.google.com/storage/) bucket hosted within a project account as is the case for the Serverless Framework.

The DevOps team recommends that project teams adopt automated continuous delivery to deploy and configure applications.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we clarify the level of support the devops team will provide for this?

Copy link
Contributor

@skuenzli skuenzli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this LGTM.

A couple of questions to consider prior to merging:

  • are 'project' and 'account' used in the canonical way for GCP?
  • we could use he- instead of helpful- for account/project prefixes; if you've already provisioned as helpful- I would leave it. using he- would align to AWS.

Comment on lines +50 to +63
helpfulengineering.org Org
└ Core
└ Log
└ Audit
└ Sandboxes
└ helpful-gcp-sandbox2
└ Project Delivery
└ Monitoring O2
└ helpful-project-monitoring-O2-dev
└ helpful-project-monitoring-O2-prod
└ <project name>
└ helpful-<project name>-dev
└ helpful-<project name>-prod
└ ...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annotating the resource hierarchy to show what types of resources are at different levels. Not sure if this is accurate or helpful, so dispose of it if not.

Suggested change
helpfulengineering.org Org
└ Core
└ Log
└ Audit
└ Sandboxes
└ helpful-gcp-sandbox2
└ Project Delivery
└ Monitoring O2
└ helpful-project-monitoring-O2-dev
└ helpful-project-monitoring-O2-prod
└ <project name>
└ helpful-<project name>-dev
└ helpful-<project name>-prod
└ ...
helpfulengineering.org (organization resource)
└ Core (folder resource)
└ Log (folder resource)
└ Audit
└ Sandboxes (folder resource)
└ helpful-gcp-sandbox2 (project resource)
└ Project Delivery (folder resource)
└ Monitoring O2 (folder resource)
└ helpful-project-monitoring-O2-dev (project resource)
└ helpful-project-monitoring-O2-prod
└ <project name> (folder resource)
└ helpful-<project name>-dev (project resource)
└ helpful-<project name>-prod
└ ...

Copy link
Contributor

@imonthercks imonthercks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Agree we need to make some changes to nomenclature about accounts vs. projects between AWS and GCP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants