Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore: Force license uuids to not be null#6012

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Emyrk merged 5 commits intomainfromstevenmasley/license_id
Feb 14, 2023
Merged

Conversation

Emyrk
Copy link
Member

@EmyrkEmyrk commentedFeb 3, 2023
edited
Loading

What this does

All new licenses have uuids, old licenses might not. So this PR gives old licenses a random uuid to support a non-null database schema.

Future work

We should require licenses to have uuids or fail. Breaking old licenses.

@Emyrk
Copy link
MemberAuthor

@spikecurtis
Copy link
Contributor

spikecurtis commentedFeb 3, 2023
edited
Loading

It's unclear to me why we needed a UUID in the first place.

We had just anid index --- is it just to normalize with other DB objects where we use UUIDs for them? I don't like the idea of having two IDs for the same thing.

@Emyrk
Copy link
MemberAuthor

@spikecurtis I am not sure. I do not think this PR is the fix, but something should be done.

@spikecurtis
Copy link
Contributor

Looks like UUIDs were added in#5110

@kylecarbs can you bring us up to speed on why UUIDs were added and whether we can drop one or other ofiduuid?

@Emyrk
Copy link
MemberAuthor

We could make the code accept invalid uuids if the license is generated before Feb 8, 2023 for example. Any new licenses have to have valid uuids.

@Kira-Pilot
Copy link
Member

@kylecarbs bump
This is blocking#6125. I would prefer it if we could stick with the UUID because the Audit Logneeds that type and seems like every other model has it.

@spikecurtis
Copy link
Contributor

@Emyrk Is it now the case that the UUID is embedded in the signed license itself, or its it just a DB identifier local to the deployment it is installed on?

@Emyrk
Copy link
MemberAuthor

It is embedded to the license claims. We insert a uuid string in the JWT ID field:https://github.com/coder/license/blob/2ae8cabd512f05b58ab871d41e6327bbc405a00d/cmd/licensor/issue.go#L79-L79

@Emyrk
Copy link
MemberAuthor

@spikecurtis uuids were added to licenses 4 months ago:https://github.com/coder/license/commit/2d26cbd6f689b566a2e84b47d98f94748e2c7fa5

I am not sure if licenses exist without uuids. It is possible. However I think we could just force the error if the license is made after a certain date to handle any old formats.

@kylecarbs
Copy link
Member

Would this stop old licenses from working@Emyrk?

@Emyrk
Copy link
MemberAuthor

@kylecarbs To support old licenses that failuuid parse, I think we would need to check the issue date (if it's in there) and allow uuid to fail for licenses before a certain date.

This PR does not do that yet.

@Emyrk
Copy link
MemberAuthor

I am going to generate a random uuid for licenses that do not have uuids.

This will make old and new licenses work. A followup should be torequire uuids, potentially breaking old licenses.

@EmyrkEmyrk merged commit733f58c intomainFeb 14, 2023
@EmyrkEmyrk deleted the stevenmasley/license_id branchFebruary 14, 2023 00:22
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsFeb 14, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@coadlercoadlercoadler approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@Emyrk@spikecurtis@Kira-Pilot@kylecarbs@coadler

[8]ページ先頭

©2009-2025 Movatter.jp