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

allow users to bring their own credentials and override MLZ Service Principal creation #315

Merged
merged 15 commits into from
Jul 29, 2021

Conversation

glennmusa
Copy link
Contributor

@glennmusa glennmusa commented Jul 23, 2021

Description

This change proposes adding a --no-service-principal flag to deploy.sh in conjunction with two environment variables ARM_CLIENT_ID and ARM_CLIENT_SECRET to deploy MLZ without creating a brand new Service Principal.

This change allows those who do not have Owner RBAC permissions but have been given Subscriptions and a Service Principal with Contributor role to those subscriptions to deploy MissionLZ.

Two new scripts were introduced with this change:

  1. checkforarmcredential.sh

This script ensures the environment variables ARM_CLIENT_ID and ARM_CLIENT_SECRET are set and that the Service Principal exists.

  1. validate_minimum_role_for_sp.sh

This scripts ensures that the user supplied Service Principal has at minimum a Contributor RBAC role scoped for each of the subscriptions the user is deploying into and provides the scripts the user would run to remediate the issue.

A few good ways to test this:

Deploy with the --no-service-principal flag to test the environment variable check:

src/scripts/deploy.sh -s $your_sub_id --no-service-principal

Create yourself a new Service Principal, set the relevant environment variables, and attempt to deploy into multiple subscriptions without assigning the roles:

az ad sp create-for-rbac
export ARM_CLIENT_ID={the resulting app ID}
export ARM_CLIENT_SECRET={the resulting password}
src/scripts/deploy.sh -s $your_sub_id -u $your_hub_id -0 $your_t0_id --no-service-principal

For more help, check the updated docs.

Issue reference

The issue this PR will close: #176

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles or validates correctly
  • BASH scripts have been validated using shellcheck
  • All tests pass (manual and automated)
  • The documentation is updated to cover any new or changed features
  • Markdown files have been linted using the recommended linter. (See .vscode/extensions.json.)
  • Relevant issues are linked to this PR

@Chambras
Copy link
Contributor

So far this has been working as expected for me in commercial and gov. I even used a different SP with owner rights which is expected to work, and it did!
I will start doing some negative testing and see how it reacts and the outputs it shows to the user, but so far it works!

@brooke-hamilton
Copy link
Contributor

So far this has been working as expected for me in commercial and gov. I even used a different SP with owner rights which is expected to work, and it did!
I will start doing some negative testing and see how it reacts and the outputs it shows to the user, but so far it works!

This also worked well for me in commercial, including failure scenarios like the SP not having the contributor role.

@glennmusa glennmusa enabled auto-merge (squash) July 29, 2021 12:40
@glennmusa glennmusa merged commit d33214b into main Jul 29, 2021
@glennmusa glennmusa deleted the glennmusa/usersuppliedprincipal branch July 29, 2021 17:48
Breanna-Stryker added a commit that referenced this pull request Aug 26, 2021
* Updated contribution process (#309)

* allow users to bring their own credentials and override MLZ Service Principal creation (#315)

* Update Terraform to version 1.0.3 (#318)

Co-authored-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com>

* updated NOTICE to remove unused licenses (#321)

* Set missing provider on Sentinel LAWS (#326)

* Update Terraform to version 1.0.4 (#334)

* update terraform required version (#336)

* Updating mlz variables file (#338)

* Update azurerm provider to 2.71.0 (#339)

* Updating tier3 variables file (#340)

* Updated issue templates (#349)

- Changed 'Issue' type to 'Backlog Item'
- Consolidated 'Feature request' and 'Enhancement' into 'Enhancement'

* add CODEOWNERS file (#364)

* Updating some modules variables files (#363)

* Add NIST policy assignment off by default (#350)

Co-authored-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com>
Co-authored-by: Glenn Musa <4622125+glennmusa@users.noreply.github.com>
Co-authored-by: Marcelo Zambrana Villarroel <marcech.am@gmail.com>
Co-authored-by: Steven St Jean <stestjea@microsoft.com>
Co-authored-by: Shawn Gibbs <shawngib@microsoft.com>
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.

Allow users to bring their own credential for deployment
3 participants