Skip to content

Conversation

@khanhtc1202
Copy link
Member

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

Add an option to allow users not belonging to any registered teams to log in

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 32.10%. This pull request increases coverage by 0.02%.

File Function Base Head Diff
pkg/oauth/github/github.go OAuthClient.decideRole 94.74% 95.45% +0.72%

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 32.10%. This pull request increases coverage by 0.02%.

File Function Base Head Diff
pkg/oauth/github/github.go OAuthClient.decideRole 94.74% 95.45% +0.72%

@khanhtc1202
Copy link
Member Author

/hold

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 32.10%. This pull request increases coverage by 0.02%.

File Function Base Head Diff
pkg/oauth/github/github.go OAuthClient.decideRole 94.74% 95.45% +0.72%

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 84.50%. This pull request increases coverage by 0.00%.

File Base Head Diff
src/fixtures/dummy-project.ts 100.00% 100.00% +0.00%

@nghialv
Copy link
Member

nghialv commented Aug 19, 2021

Nice adding.
/lgtm

@pipecd-bot pipecd-bot added area/api and removed lgtm labels Aug 19, 2021
@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 32.10%. This pull request increases coverage by 0.02%.

File Function Base Head Diff
pkg/oauth/github/github.go OAuthClient.decideRole 94.74% 95.45% +0.72%

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 84.50%. This pull request increases coverage by 0.00%.

File Base Head Diff
src/fixtures/dummy-project.ts 100.00% 100.00% +0.00%

@khanhtc1202
Copy link
Member Author

/hold cancel

@khanhtc1202
Copy link
Member Author

khanhtc1202 commented Aug 19, 2021

@nghialv @nakabonne I moved the previous flag ProjectRBACConfig.viewer_role_as_default to Project.allow_stray_as_viewer since in case we adopt the ProjectRBACConfig.viewer_role_as_default, we need to add value for ProjectRBACConfig.admin field (it's required on project rbac creation - ref project.proto). After thinking twice, I guess we should keep the project creation form as simple as possible (should not force operators to config their admin team on creation, keep it as simple as we do now), so moving the above flag to Project.allow_stray_as_viewer is a good option, PTAL 🙏

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 32.10%. This pull request increases coverage by 0.02%.

File Function Base Head Diff
pkg/oauth/github/github.go OAuthClient.decideRole 94.74% 95.45% +0.72%

@nakabonne
Copy link
Member

/lgtm

<label>Shared SSO</label>
<input type="text" name="SharedSSO"><br><br>
<input type="checkbox" id="auto-assign" name="ViewerRoleAsDefault">
<label for="auto-assign">Auto assign viewer role</label><br><br>
Copy link
Member

Choose a reason for hiding this comment

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

Fix the name and label around this part.

id = r.FormValue("ID")
description = r.FormValue("Description")
sharedSSOName = r.FormValue("SharedSSO")
viewerRoleAsDefault = r.FormValue("ViewerRoleAsDefault") == "on"
Copy link
Member

Choose a reason for hiding this comment

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

Fix this name too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch, addressed on b9eb3fe 🙏

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 84.45%. This pull request decreases coverage by -0.05%.

File Base Head Diff
src/fixtures/dummy-project.ts 100.00% 100.00% +0.00%
src/modules/project/index.ts 60.38% 56.60% -3.77%

@khanhtc1202 khanhtc1202 changed the title Add viewerRoleAsDefault option Add AllowStrayAsViewer option Aug 19, 2021
@pipecd-bot pipecd-bot removed the lgtm label Aug 19, 2021
@nghialv
Copy link
Member

nghialv commented Aug 19, 2021

Thank you.
/approve

@pipecd-bot
Copy link
Collaborator

APPROVE

This pull request is APPROVED by nghialv.

Approvers can cancel the approval by writing /approve cancel in a comment. Any additional commits also will change this pull request to be not-approved.

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 32.10%. This pull request increases coverage by 0.02%.

File Function Base Head Diff
pkg/oauth/github/github.go OAuthClient.decideRole 94.74% 95.45% +0.72%

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 84.50%. This pull request increases coverage by 0.00%.

File Base Head Diff
src/fixtures/dummy-project.ts 100.00% 100.00% +0.00%

@pipecd-bot pipecd-bot merged commit bcb1483 into master Aug 19, 2021
@pipecd-bot pipecd-bot deleted the add-viewer-as-default branch August 19, 2021 08:47
@khanhtc1202
Copy link
Member Author

/changelog

@pipecd-bot
Copy link
Collaborator

CHANGELOG

@khanhtc1202: Changelog has been generated in response to this comment.

Details

Changelog since v0.14.0

Notable Changes

  • Add an option to allow users not belonging to any registered teams to log in (#2370)
  • Add a new ops job to clean stale plan-preview outputs (#2356)
  • Add text regex support for Event watcher (#2347)

Internal Changes

  • Update documentation for monitoring (#2366)
  • Update ops to run planpreview cleaner at a fixed time (#2368)
  • Add project label to piped metrics (#2367)
  • Add Event watcher example using regex (#2365)
  • Add settings to talk to Alertmanager (#2364)
  • Fix typo on helm template (#2363)
  • Add Cloud SQL Proxy as server and ops sidecar (#2362)
  • Add sql service account (#2361)
  • Disallow to update Grafana UI from the web (#2360)
  • Fix a typo (#2359)
  • Update lambda deployment feature status (#2358)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants