[A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs, add `--skip-policy-migration` to `upgrade-to-v2` #403

srenatus · 2019-05-22T09:47:52Z

🔩 Description

Our environments, both for local development and dev/acceptance, are less useful than they could be if everyone keeps using the admin user for all the things. Especially in the short-lived ones, it's cumbersome to add these users manually for acceptance testing time and time again.

The same goes for the local development environment. The world looks quite different if you're not an admin 😉

All of this only applies to envs using IAM v2 or v2.1. However, the users can exist in v1 environments in just the same way, so the local studio env is not upgraded to IAM v2 or IAM v2.1 yet (as of now). ✔️ local studio env is upgrade as well.

Reviewers!

Please follow the commits. There's a few of them, and they should tell the story of this PR 😉

👍 Definition of Done

In all used environments (IAM v2 or IAM v2.1), there are these two default users, members of their corresponding teams (team:local:viewers, team:local:editors)
the a2-iam-v2-only-integration inspec suite no longer creates the users itself, but relies on the newly added cli command for that
there's a (hidden) flag to chef-automate iam upgrade-to-v2, --skip-legacy-upgrade, that causes v1 policies to not get migrated
that flag is used for our short-lived dev/acceptance envs
that flag is used in start_all_services
rename the flag to --skip-policy-migration for clarity (discussed this with @susanev)

👟 Demo Script / Repro Steps

local development

rebuild components/automate-cli
rebuild components/automate-gateway, rebuild components/authz-service for testing chef-automate iam update-to-v2 --skip-policy-migration
start_all_services
login as either "viewer" or "editor" (password is "chefautomate")
to independently check the --skip-policy-migration flag's output:

[78][default:/src:0]# chef-automate iam reset-to-v1

Reverting to IAM v1 (with pre-upgrade v1 policies)
[79][default:/src:0]# chef-automate iam upgrade-to-v2 --beta2.1 --skip-policy-migration

Enabling IAM v2.1

Skipped policies:
  28 v1 policies
Creating default teams Editors and Viewers...
Skipped: Editors team already exists
Skipped: Viewers team already exists

Migrating existing teams...

Success: Enabled IAM v2.1
[80][default:/src:0]#

note that for now, the skip flag is unhidden in the dev env (studio):

[8][default:/src:0]# chef-automate iam upgrade-to-v2 --help
Upgrade to IAM v2 and migrate existing v1 policies. On downgrade, any new v2 policies will be reverted.

Usage:
  chef-automate iam upgrade-to-v2 [flags]

Flags:
      --beta2.1                 Upgrade to version 2.1 with beta project authorization.
  -h, --help                    help for upgrade-to-v2
      --skip-policy-migration   Do not migrate policies from IAM v1.

Global Flags:
  -d, --debug                Enable debug output
      --no-check-version     Disable version check
      --result-json string   Write command result as JSON to PATH

to simulate outside-of-dev behaviour, set CHEF_DEV_ENVIRONMENT to something that's not yes:

[80][default:/src:0]# env CHEF_DEV_ENVIRONMENT=no chef-automate iam upgrade-to-v2 --help
Upgrade to IAM v2 and migrate existing v1 policies. On downgrade, any new v2 policies will be reverted.

Usage:
  chef-automate iam upgrade-to-v2 [flags]

Flags:
  -h, --help   help for upgrade-to-v2

Global Flags:
  -d, --debug                Enable debug output
      --no-check-version     Disable version check
      --result-json string   Write command result as JSON to PATH

dev/acceptance envs

let's check that post-merge and post-promotion 🤞

⛓️ Related Resources

A2-572

✅ Checklist

Necessary tests added/updated?
Necessary docs added/updated?
Code actually executed?
Vetting performed (unit tests, lint, etc.)?

Reusing the logic we've had for resetting admin access. Signed-off-by: Stephan Renatus <[email protected]>

This hooks in the call to the new dev command. The resulting logins will be: - viewer:chefautomate - editor:chefautomate Signed-off-by: Stephan Renatus <[email protected]>

Signed-off-by: Stephan Renatus <[email protected]>

…tion Signed-off-by: Stephan Renatus <[email protected]>

Signed-off-by: Stephan Renatus <[email protected]>

…buf] Signed-off-by: Stephan Renatus <[email protected]>

Signed-off-by: Stephan Renatus <[email protected]>

…otobuf] Signed-off-by: Stephan Renatus <[email protected]>

Signed-off-by: Stephan Renatus <[email protected]>

…icies Signed-off-by: Stephan Renatus <[email protected]>

Signed-off-by: Stephan Renatus <[email protected]>

Since this call only happens once (the second time it's run, the existance of the a2-iamv2-enabled file will block execution), it will only affect our short-lived environments. Should we ever re-deploy our long-lived ones, then this _would_ also skip their legacy policies. However, I'm not sure why we'd like to avoid that. Signed-off-by: Stephan Renatus <[email protected]>

Now, the zero value reflects the current behaviour. Without this, existing caller (tests!) would have requested the v1 policies to be ignored. Signed-off-by: Stephan Renatus <[email protected]>

Signed-off-by: Stephan Renatus <[email protected]>

git grep skip-legacy-upgrade |\ gawk -F: '!a[$1]++{ print $1 }' |\ xargs -- gsed -i s/skip-policy-upgrade/skip-policy-migration/g Signed-off-by: Stephan Renatus <[email protected]>

Signed-off-by: Stephan Renatus <[email protected]>

msorens · 2019-05-23T13:41:16Z

components/automate-cli/cmd/chef-automate/iam.go

-	}
+
+	// all flags are hidden right now
+	cmd.PersistentFlags().VisitAll(func(f *pflag.Flag) { f.Hidden = !isDevMode() })


I thought the team consensus was that skip-policy-migration was for public consumption as well...?

i believe that consensus convo happened last friday during subtasking and i wasnt there. if we do make it public we should have a corresponding docs update, so id like to do that in a separate pr if possible.

I'm all in favour of not adding anything here 😉

msorens · 2019-05-23T15:46:51Z

components/authz-service/server/v2/policy.go

+			recordFailure()
+			return nil, status.Errorf(codes.Internal, "list v1 policies: %s", err.Error())
+		}
+		reports = append(reports, fmt.Sprintf("%d v1 policies", len(pols)))


Consider "%d v1 policies found -- skipping"

The output, in the end, will be

[79][default:/src:0]# chef-automate iam upgrade-to-v2 --beta2.1 --skip-policy-migration Enabling IAM v2.1 Skipped policies: 28 v1 policies

(see PR message) -- I'd rather not use skipped twice there

@srenatus do you mind if we have a single line with Skipped: %d v1 policies, then a newline before the teams output?

Right now, that's shared between a migration and no migration (see https://github.com/chef/a2/pull/4120#issuecomment-433389839), so it's a little more work than just the formatting. But yeah, we can do that.

oh i see.... nah, just leave it as is for now.

msorens · 2019-05-23T15:49:15Z

components/automate-cli/cmd/chef-automate/dev.go

+			return err
+		}
+		// Note: the teams SHOULD exist. But since you never know what happens in a
+		// long running acceptance env, we'll better ensure them:


s/we'll/we'd/

msorens · 2019-05-23T15:52:52Z

components/automate-cli/cmd/chef-automate/dev.go

+		"editor": {"Editor User", "chefautomate", "editors"},
+	} {
+		userID, _, err := adminmgmt.CreateUserOrUpdatePassword(ctx,
+			apiClient, username, data.displayName, data.password, false /* dry run */)


Personal style thing for sure, but just my 2 cents:
false /* dry run */ suggests to me that "setting this param false here and that means it is a dry run".
Whereas:
/* dry run */ false suggests to me that "this is supplying a value of false for 'dry run' and that means it is not a dry run.

hehe I didn't mean to convey that much -- I really just meant "this bool is the dry run parameter". But yeah, you're not wrong with that connotation 😉

msorens · 2019-05-23T16:07:03Z

...m/test-environments/modules/chef_automate_install/templates/install_chef_automate_cli.sh.tpl

      touch /root/a2-iamv2-enabled
      ;;
    "v2")
-      chef-automate iam upgrade-to-v2
+      chef-automate iam upgrade-to-v2 --skip-policy-migration
+      chef-automate dev create-iam-dev-users


Good stuff!

srenatus · 2019-05-24T16:04:13Z

I'll merge this on Monday and deal with the fallout.

srenatus added WIP studio-dev labels May 22, 2019

srenatus requested a review from a team as a code owner May 22, 2019 09:47

srenatus self-assigned this May 22, 2019

srenatus force-pushed the sr/a2-572/add-default-users-creation-to-automate-cli-dev branch from bae9a81 to 38cca3c Compare May 22, 2019 11:12

srenatus added the automate-auth label May 22, 2019

srenatus changed the title ~~[A2-572] WIP~~ [A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs May 22, 2019

srenatus changed the title ~~[A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs~~ [A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs, add --skip-legacy-upgrade to upgrade-to-v2 May 22, 2019

srenatus force-pushed the sr/a2-572/add-default-users-creation-to-automate-cli-dev branch 3 times, most recently from 563cdfa to cc93a17 Compare May 22, 2019 15:32

srenatus added 19 commits May 23, 2019 11:29

automate-cli/dev: add cmd to create IAM dev users (viewer, editor)

fd53817

Reusing the logic we've had for resetting admin access. Signed-off-by: Stephan Renatus <[email protected]>

start_all_services: create IAM v2 dev users

5d928c9

This hooks in the call to the new dev command. The resulting logins will be: - viewer:chefautomate - editor:chefautomate Signed-off-by: Stephan Renatus <[email protected]>

terraform: create iam dev users in test envs

e66dde3

Signed-off-by: Stephan Renatus <[email protected]>

automate-cli/dev: mention that the command can be run many times

d448ad2

Signed-off-by: Stephan Renatus <[email protected]>

inspec/a2-iam-v2-only-integration: rely on automate-cli for user crea…

6f084f7

…tion Signed-off-by: Stephan Renatus <[email protected]>

automate-cli/iam: add --skip-legacy-upgrade flag (hidden)

7230d15

Signed-off-by: Stephan Renatus <[email protected]>

automate-gateway/api: add migrate_v1_policies to UpdateToV2Req

35e1d6e

Signed-off-by: Stephan Renatus <[email protected]>

automate-gateway/api: add migrate_v1_policies to UpdateToV2Req [proto…

a3f140b

…buf] Signed-off-by: Stephan Renatus <[email protected]>

automate-cli/iam: set migrate_v1_policies according to provided flag

1fee8f2

Signed-off-by: Stephan Renatus <[email protected]>

api/interservice/authz: add migrate_v1_policies to MigrateToV2Req

2861676

Signed-off-by: Stephan Renatus <[email protected]>

api/interservice/authz: add migrate_v1_policies to MigrateToV2Req [pr…

d0bedb7

…otobuf] Signed-off-by: Stephan Renatus <[email protected]>

automate-gateway/authz: pass migrate_v1_policies along

a94090c

Signed-off-by: Stephan Renatus <[email protected]>

authz-service/migration: nitpicks

5cbf498

Signed-off-by: Stephan Renatus <[email protected]>

authz-service/upgrade-to-v2: respect migrate_v1_policies flag

1ce7045

Signed-off-by: Stephan Renatus <[email protected]>

authz-service/upgrade-to-v2: report number of skipped policies, not IDs

2501bfe

Signed-off-by: Stephan Renatus <[email protected]>

integration/tests/iam_v2p1_only: rely on skip flag to skip legacy pol…

f745c68

…icies Signed-off-by: Stephan Renatus <[email protected]>

studio: upgrade to IAM v2 in start_all_services

c65a147

Signed-off-by: Stephan Renatus <[email protected]>

api: flip migrate_v1_policies -> skip_v1_policies

613f37f

Now, the zero value reflects the current behaviour. Without this, existing caller (tests!) would have requested the v1 policies to be ignored. Signed-off-by: Stephan Renatus <[email protected]>

srenatus added 4 commits May 23, 2019 11:29

api: flip migrate_v1_policies -> skip_v1_policies [protobuf]

7fa7955

Signed-off-by: Stephan Renatus <[email protected]>

api: flip migrate_v1_policies -> skip_v1_policies (adapt code)

49ce217

Signed-off-by: Stephan Renatus <[email protected]>

authz-service/migration: add test case

aeeed92

Signed-off-by: Stephan Renatus <[email protected]>

studiorc: free a cat

8c2849f

Signed-off-by: Stephan Renatus <[email protected]>

srenatus force-pushed the sr/a2-572/add-default-users-creation-to-automate-cli-dev branch from cc93a17 to 8d4e9e7 Compare May 23, 2019 09:33

rename flag: --skip-policy-migration

61ccc53

git grep skip-legacy-upgrade |\ gawk -F: '!a[$1]++{ print $1 }' |\ xargs -- gsed -i s/skip-policy-upgrade/skip-policy-migration/g Signed-off-by: Stephan Renatus <[email protected]>

srenatus force-pushed the sr/a2-572/add-default-users-creation-to-automate-cli-dev branch from 8d4e9e7 to 61ccc53 Compare May 23, 2019 09:35

srenatus changed the title ~~[A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs, add --skip-legacy-upgrade to upgrade-to-v2~~ [A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs, add --skip-policy-migration to upgrade-to-v2 May 23, 2019

srenatus removed the WIP label May 23, 2019

automate-cli/iam: unhide upgrade-to-v2 flags in dev mode

922ebd0

Signed-off-by: Stephan Renatus <[email protected]>

msorens reviewed May 23, 2019

View reviewed changes

susanev approved these changes May 23, 2019

View reviewed changes

msorens approved these changes May 24, 2019

View reviewed changes

srenatus merged commit 99e3425 into master May 27, 2019

chef-ci deleted the sr/a2-572/add-default-users-creation-to-automate-cli-dev branch May 27, 2019 06:26

susanev added the auth-team anything that needs to be on the auth team board label Jul 20, 2019

ekmixon mentioned this pull request Dec 1, 2023

[Snyk] Fix for 1 vulnerabilities ekmixon/automate#145

Open

ekmixon mentioned this pull request Dec 20, 2023

[Snyk] Fix for 29 vulnerabilities ekmixon/automate#149

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs, add `--skip-policy-migration` to `upgrade-to-v2` #403

[A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs, add `--skip-policy-migration` to `upgrade-to-v2` #403

srenatus commented May 22, 2019 •

edited

Loading

msorens May 23, 2019

susanev May 23, 2019

srenatus May 23, 2019

msorens May 23, 2019

srenatus May 23, 2019

susanev May 23, 2019 •

edited

Loading

srenatus May 23, 2019

susanev May 23, 2019 •

edited

Loading

msorens May 23, 2019

msorens May 23, 2019

srenatus May 23, 2019

msorens May 23, 2019

srenatus commented May 24, 2019

[A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs, add --skip-policy-migration to upgrade-to-v2 #403

[A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs, add --skip-policy-migration to upgrade-to-v2 #403

Conversation

srenatus commented May 22, 2019 • edited Loading

🔩 Description

Reviewers!

👍 Definition of Done

👟 Demo Script / Repro Steps

local development

dev/acceptance envs

⛓️ Related Resources

✅ Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

susanev May 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

susanev May 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srenatus commented May 24, 2019

[A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs, add `--skip-policy-migration` to `upgrade-to-v2` #403

[A2-572] Add 'viewer' and 'editor' users to local-dev and dev/acceptance envs, add `--skip-policy-migration` to `upgrade-to-v2` #403

srenatus commented May 22, 2019 •

edited

Loading

susanev May 23, 2019 •

edited

Loading

susanev May 23, 2019 •

edited

Loading