Updating a few toolings for orgs to use slug over IDs to be a bit more compatible with current changes #258
Conversation
Fixes #192 ChangeLog: - Drop support for import/export keywords - Drop references to legacy 'datasources' - Drop Alert Notifications as it's long been deprecated.
Fixes #254 ChangeLog: gdg: - naming convention has shifted from org_<id> to org<name|slug> - organization_name replaces organization_id gdg-generate: - org_id deprecated in favor of organization_name
safaci2000
force-pushed
the
feature/moreOrgFixes
branch
from
c0679e4
to
0db1f16
Compare
safaci2000
force-pushed
the
feature/moreOrgFixes
branch
from
0db1f16
to
0558ed1
Compare
| passLength := rand.IntN(u.MaxLength-u.MinLength) + u.MinLength | ||
| res, err := password.Generate(passLength, 1, 1, false, false) | ||
| if err != nil { | ||
| slog.Warn("unable to generate a proper random password, falling back on default password pattern", |
There was a problem hiding this comment.
Under what circumstances is generating a password going to fail? This feels like possibly extra complication / unnecessary? If there's a reason like a setting where someone can set min length > max length or something that would cause it to fail, we should detect and fatal on that earlier I think.
There was a problem hiding this comment.
Yes, those values are coming from a configuration file. So they could set say a negative min or a max of 65K or what not that probably will cause the password generator to return an error.
The two patterns are to take a sha256 of the login.json (ie file name) or create something random that won't be deterministic
There was a problem hiding this comment.
The alternative would be to just critically fail the entire process. Rather than say do a best effort
There was a problem hiding this comment.
My gut feeling here is that having a single approach is better - make this approach deterministic. It's also simply fewer code paths, things to test, things that can break, etc. If we can't make a password because someone set it up wrong, that's on them.
There was a problem hiding this comment.
Maybe. There's benefits to both. Random means you will only ever see the list once when you create them. You need to save that info otherwise it's lost.
The previous pattern was a sha256 of the login.json which is deterministic but that also means it's a security vulnerability if someone doesn't change the passwords. So having a feature to allow for random passwords is better in that regard. I think both approaches are valid, it just depends on your use case/need.
I'm not sure how used the feature is, but this would be tied to a major release ie. 0.6.X. Breaking changes are expected over the incremental releases we've been doing. It's also why the legacy configuration was removed and dropping support for import/export etc. The fact that IDs are non-reproduceable across environment makes using the ID very silly as far as patterns go. Unless you create two ENVs via DB dump, it's a roll of the die if two orgs with the same name will have the same ID or not. the slug/Org Name is a lot easier to keep consistent across environments. |
|
I agree that doing it by name is more portable and more human friendly anyway. If we're doing a version bump like this I suspect it's fine, though we'll want to make sure we call attention to it. |
|
Yup. It'll be called out in the release notes. I wish the Org had a UID or such, that would allow us to reference those, but the name is more human readable. It's just susceptible to renames and such. If you rename the Org you need to fix the config. |
Fixes #192 and #254
ChangeLog:
gdg:
gdg-generate: