MM-60036: License validation #832
Open
+61
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is to make testing easier. We can read the file outside of the function, no big deal.
A common error when providing a license is to use a different service environment (dev, test or production) to the one the license was generated for. This error is not surfaced by the call to ValidateLicense, so we explicitly test a different environment to the one currently in use to check whether license would be valid there. If that's the case, then we return a much more helpful error to the user.
Double-check that the error returned in case of a mismatch in the service environment is the one we expect.
agnivade
reviewed
Oct 16, 2024
deployment/terraform/create.go
Outdated
Comment on lines
112
to
117
| licenseData, err := os.ReadFile(t.config.MattermostLicenseFile) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to read license file: %w", err) | ||
| } | ||
|
|
||
| if err := validateLicense(licenseData); err != nil { |
There was a problem hiding this comment.
Reading from the file still seems like the right way for me. We can tweak the tests to move the license contents to a test asset file and read from it.
There was a problem hiding this comment.
I don't really see any more value in one or the other alternative, to be honest. I usually use "easier for testing" as a proxy for slightly better code, but it doesn't really matter where to get the license data from, a string or an asset file.
So I'll revert and use an asset file, then :)
cpoile
approved these changes
Oct 16, 2024
agnivade
approved these changes
Oct 16, 2024
|
cc @streamer45 as FYI |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
A common error when providing a license is to use a different service environment (dev, test or production) to the one the license was generated for. This error is not surfaced by the call to
validator.ValidateLicense, so we explicitly test a different environment to the one currently in use to check whether the license would be valid there. If that's the case, then we return a much more helpful error to the user.Note that the license in the test is not valid for production, only for test (and for 1 user just in case).
Ticket Link
https://mattermost.atlassian.net/browse/MM-60036