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.

Sign up for GitHub

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

Jump to bottom

feat: Add ADR for common authentication strategy for applications #21

Merged
merged 1 commit into from
Jan 25, 2021
Merged

feat: Add ADR for common authentication strategy for applications #21

merged 1 commit into from
Jan 25, 2021

Conversation

tumido
Copy link
Member

@tumido tumido commented Jan 19, 2021

Resolves: #9 open-infrastructure-labs/ops-issues#10

Proposes the standalone Dex server manual (not operated) deployment as a common application authentication and identity provider bridge.

@sesheta sesheta added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 19, 2021
@tumido tumido mentioned this pull request Jan 19, 2021
Closed
@tumido
Copy link
Member Author

tumido commented Jan 19, 2021

/hold

@sesheta sesheta added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 19, 2021
@HumairAK HumairAK added this to In Review in Master Board Jan 20, 2021
@HumairAK HumairAK removed this from In Review in Master Board Jan 20, 2021
@tumido tumido mentioned this pull request Jan 22, 2021
Merged
@HumairAK
Copy link
Member

Great write-up!

This LGTM besides the nit picks above, feel free to unhold this pr whenever you are ready.

@tumido tumido requested a review from HumairAK January 25, 2021 12:46
@tumido
feat: Add ADR for common authentication strategy for applications
246e677 
Signed-off-by: Tomas Coufal <tcoufal@redhat.com>
@tumido
Copy link
Member Author

tumido commented Jan 25, 2021

Updated. Please review once more @HumairAK @anishasthana 🙂

@HumairAK
Copy link
Member

lgtm
@anishasthana once you give it another look, please unhold.

@anishasthana
Copy link
Member

/unhold

@sesheta sesheta removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 25, 2021
@tumido
Copy link
Member Author

tumido commented Jan 25, 2021

@HumairAK I think you would need to approve this once more. 🙂

@sesheta
Copy link
Member

sesheta commented Jan 25, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: anishasthana, HumairAK
To complete the pull request process, please assign durandom after the PR has been reviewed.
You can assign the PR to them by writing /assign @durandom in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tumido tumido merged commit 1ad2ea4 into operate-first:main Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anishasthana anishasthana anishasthana approved these changes

@HumairAK HumairAK HumairAK approved these changes

@harshad16 harshad16 Awaiting requested review from harshad16

@4n4nd 4n4nd Awaiting requested review from 4n4nd

@durandom durandom Awaiting requested review from durandom

@hemajv hemajv Awaiting requested review from hemajv

Assignees
No one assigned
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ADR required for authentication
4 participants
@tumido @HumairAK @anishasthana @sesheta