Auth 1840/configure project limit #2029

blakestier · 2019-10-29T15:44:35Z

🔩 Description: What code changed, and why?

⛓️ Related Resources

👍 Definition of Done

👟 How to Build and Test the Change

rebuild components/authz-service
rebuild components/automate-deployment
rebuild components/automate-cli

touch authz.toml

authz.toml contents:

[auth_z.v1.sys.service]
project_limit = 7

chef-automate config patch authz.toml

Attempt to make more than 7 custom projects 💁(curl -XPOST -d '{"name":"test", "id":"foo1-project"}' -H "api-token: $TOK" $TARGET_HOST/apis/iam/v2beta/projects --insecure)

✅ Checklist

Tests added/updated?
Docs added/updated?

📷 Screenshots, if applicable

bcmdarroch

Code looks good so far! I would add a few more tests.

After rebuilding authz and deployment I tried to exercise it with these commands (from #1840) but either my toml is wrong or I haven't rebuilt something.

[8][default:/src:93]# cat <<EOF > authz2.toml
[authz.v1.sys.svc]
project_limit = 300
EOF

[9][default:/src:0]# chef-automate config patch authz2.toml
ConfigError: The configuration is invalid: Config file must be a valid automate config: unknown configuration keys:
    authz.v1.sys.svc
    authz.v1.sys.svc.project_limit

bcmdarroch · 2019-10-29T19:36:40Z

components/authz-service/storage/v2/postgres/postgres_test.go

@@ -4410,7 +4410,9 @@ func TestCreateProject(t *testing.T) {
 			}
 			resp, err := store.CreateProject(ctx, &oneProjectTooMany)
 			assert.Nil(t, resp)
-			assert.Equal(t, storage_errors.ErrMaxProjectsExceeded, err)
+			assert.Error(t, err)


could we add a test for configuring a different value for the project limit?

~~we can't without refactoring the whole test suite :D but i'll do it for you if you want~~

lol it was not bad 😂

api/config/deployment/automate_config_test.go

susanev · 2019-10-29T19:57:38Z

yah i also couldnt get that patch to work, but ive never tried to do that before either so maybe im doing something wrong

api/config/authz/config_request.go

msorens

Looks good! Just a few minor suggestions.

msorens · 2019-10-30T17:00:11Z

.studio/tooling

@@ -136,3 +136,15 @@ function install_go_tool() {
    fi
  done
 }
+
+document "revendor" <<DOC


What does this do?

@msorens I pulled this in from #2039. Needed it to successfully update the bldr.toml

msorens · 2019-10-30T17:03:15Z

api/config/authz/config_request.go

+
+	if projectLimit != nil {
+		if limit := projectLimit.GetValue(); limit < constants_v2.MaxProjects {
+			e := errors.Errorf("invalid project limit of %v: must be minimum of %v", limit, constants_v2.MaxProjects)


Using "minimum" to describe a maximum seems confusing. Also, consider using a shorter phrase, e.g., just:

"project limit must be at least %v"

msorens · 2019-10-30T17:07:20Z

components/authz-service/storage/errors.go

+// MaxProjectsExceededError indicates that a new project cannot be created
+// since the max allowed are already created.
+func (e *MaxProjectsExceededError) Error() string {
+	return fmt.Sprintf("violates project limit of %v", e.projectLimit)


Not clear what context is provided along with this text. But assuming this is the entirety of what the user sees, consider including the info in the comment on L83-84, e.g.

"cannot create project: limit of %v projects already reached"

Signed-off-by: Blake Johnson <bstier@chef.io>

Signed-off-by: Ryan Cragun <ryan@chef.io>

Signed-off-by: Blake Johnson <bstier@chef.io>

Signed-off-by: Ryan Cragun <ryan@chef.io>

Signed-off-by: Blake Johnson <bstier@chef.io>

bcmdarroch

sweet 👍 👍

sdelano · 2019-11-01T00:15:11Z

closes #2076

susanev assigned blakestier Oct 29, 2019

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

blakestier marked this pull request as ready for review October 29, 2019 19:02

susanev mentioned this pull request Oct 29, 2019

Allow Admin to Configure Project Limit #1840

Closed

9 tasks

bcmdarroch reviewed Oct 29, 2019

View reviewed changes

blakestier force-pushed the auth-1840/configure-project-limit branch 3 times, most recently from 8006a7c to e5fa1ed Compare October 29, 2019 22:24

ryancragun reviewed Oct 29, 2019

View reviewed changes

api/config/authz/config_request.go Outdated Show resolved Hide resolved

blakestier force-pushed the auth-1840/configure-project-limit branch 2 times, most recently from 02ef144 to 0e7bbb0 Compare October 30, 2019 15:23

msorens approved these changes Oct 30, 2019

View reviewed changes

Blake Johnson and others added 12 commits October 30, 2019 10:46

Update config to accept project limit

ccb137e

Signed-off-by: Blake Johnson <bstier@chef.io>

Validate project limit

d9eb046

Signed-off-by: Blake Johnson <bstier@chef.io>

Initialize pg with project limit

27da5ae

Signed-off-by: Blake Johnson <bstier@chef.io>

Use config value to enforce limit

e2a0c89

Signed-off-by: Blake Johnson <bstier@chef.io>

Update test

7dda13e

Signed-off-by: Blake Johnson <bstier@chef.io>

Allow max proj error to print limit

d54a195

Signed-off-by: Blake Johnson <bstier@chef.io>

Memstore limit aware

643725b

Signed-off-by: Blake Johnson <bstier@chef.io>

[studio] add revendor support

0cb558d

Signed-off-by: Ryan Cragun <ryan@chef.io>

Allow config setting in test

a801ee1

Signed-off-by: Blake Johnson <bstier@chef.io>

Update bldr.toml

a4b49ce

Signed-off-by: Blake Johnson <bstier@chef.io>

Deploy test fix

0aeef96

Signed-off-by: Blake Johnson <bstier@chef.io>

[config] only validate project config when user has set it

45ab852

Signed-off-by: Ryan Cragun <ryan@chef.io>

blakestier force-pushed the auth-1840/configure-project-limit branch from 0b475e8 to bd60e4f Compare October 30, 2019 17:46

Modify error msg

2f1fd72

Signed-off-by: Blake Johnson <bstier@chef.io>

blakestier force-pushed the auth-1840/configure-project-limit branch from bd60e4f to 2f1fd72 Compare October 30, 2019 17:59

blakestier mentioned this pull request Oct 30, 2019

Remove project limit from UI #2049

Merged

bcmdarroch approved these changes Oct 30, 2019

View reviewed changes

blakestier merged commit 7bb175e into master Oct 30, 2019

blakestier deleted the auth-1840/configure-project-limit branch October 30, 2019 20:31

bcmdarroch mentioned this pull request Oct 30, 2019

[automate-1840] bump the project limit on dev/acceptance environments #2051

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auth 1840/configure project limit #2029

Auth 1840/configure project limit #2029

blakestier commented Oct 29, 2019 •

edited

Loading

bcmdarroch left a comment

bcmdarroch Oct 29, 2019

blakestier Oct 29, 2019 •

edited

Loading

susanev commented Oct 29, 2019

msorens left a comment

msorens Oct 30, 2019

blakestier Oct 30, 2019

msorens Oct 30, 2019

msorens Oct 30, 2019

bcmdarroch left a comment

sdelano commented Nov 1, 2019

Auth 1840/configure project limit #2029

Auth 1840/configure project limit #2029

Conversation

blakestier commented Oct 29, 2019 • edited Loading

🔩 Description: What code changed, and why?

⛓️ Related Resources

👍 Definition of Done

👟 How to Build and Test the Change

✅ Checklist

📷 Screenshots, if applicable

bcmdarroch left a comment

Choose a reason for hiding this comment

bcmdarroch Oct 29, 2019

Choose a reason for hiding this comment

blakestier Oct 29, 2019 • edited Loading

Choose a reason for hiding this comment

susanev commented Oct 29, 2019

msorens left a comment

Choose a reason for hiding this comment

msorens Oct 30, 2019

Choose a reason for hiding this comment

blakestier Oct 30, 2019

Choose a reason for hiding this comment

msorens Oct 30, 2019

Choose a reason for hiding this comment

msorens Oct 30, 2019

Choose a reason for hiding this comment

bcmdarroch left a comment

Choose a reason for hiding this comment

sdelano commented Nov 1, 2019

blakestier commented Oct 29, 2019 •

edited

Loading

blakestier Oct 29, 2019 •

edited

Loading