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

Static Users #1755

Closed
wants to merge 10 commits into from
Closed

Static Users #1755

wants to merge 10 commits into from

Conversation

luizbafilho
Copy link
Contributor

@luizbafilho luizbafilho commented Mar 18, 2022

Closes: #1598 #1722

What changed?
The concept of Static User has been introduced. This feature allows operators to create an undefined number of Users by creating K8s Secrets with a well-defined structure and label so that Weave Gitops can query and match credentials.

Why?

  • Allow configuration flexibility to operators.

How did you test it?

  • Manual and automated tests validate the flows.

Release notes

*Documentation Changes

@luizbafilho luizbafilho changed the title Tweaking admin password Static Users Mar 18, 2022
@luizbafilho luizbafilho added the type/enhancement New feature or request label Mar 18, 2022
@luizbafilho luizbafilho marked this pull request as ready for review March 22, 2022 14:53
@luizbafilho luizbafilho requested a review from Callisto13 March 22, 2022 14:53
@luizbafilho
Copy link
Contributor Author

FYI @yiannistri

@luizbafilho luizbafilho requested a review from jpellizzari March 22, 2022 14:55
Copy link
Contributor

@Callisto13 Callisto13 left a comment

Choose a reason for hiding this comment

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

Really awesome! I just have one question

if loginRequest.Username != string(hashedSecret.Data["username"]) {
s.Log.Info("Wrong username")
rw.WriteHeader(http.StatusUnauthorized)
if err := s.kubernetesClient.List(r.Context(), users, opts...); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to list all users, or can we just get a specific one based on the username?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can't query based on any data value. So to accomplish that we would need to force users to use some kind of convention for names, but that would still be tricky because they can choose to use an email address as the username and that's not a valid name for a k8s resource.

Listing isn't ideal but at least we only list the secrets with that particular Label. And in practice, there should be a small number of those.

Copy link
Contributor

Choose a reason for hiding this comment

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

aah i see, makes sense

Copy link
Contributor

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

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

I don't fully grasp the context here, but no issues with the code.

@Callisto13
Copy link
Contributor

This may be being rolled back, see: https://weaveworks.slack.com/archives/C03244W0C8H/p1647967858091769

@Callisto13
Copy link
Contributor

#1787

@makkes makkes deleted the 1671-static-users branch December 9, 2022 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement a Dev mode
3 participants