-
Notifications
You must be signed in to change notification settings - Fork 27
Add validation to only allow Cassandra major version 3 #321
Changes from 7 commits
304bb3c
0418c2b
2f7eb7e
4c365f9
67450fe
f284870
b034203
930bf6f
635c5fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,9 @@ package version | |
|
|
||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "strconv" | ||
| "strings" | ||
|
|
||
| "github.com/coreos/go-semver/semver" | ||
| semver "github.com/hashicorp/go-version" | ||
| ) | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we move this file out of It'd be good to denote it as such in the file structure, to make it clear this isn't specific to just cassandra (and also denotes that this type may be exposed in an api)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| // Version represents a Cassandra database server version. | ||
|
|
@@ -23,9 +21,11 @@ import ( | |
| // This also fixes the missing Patch number and stores the version internally as a semver. | ||
| // It also keeps a reference to the original version string so that we can report that in our API. | ||
| // So that the version reported in our API matches the version that an administrator expects. | ||
| // | ||
| // +k8s:deepcopy-gen=true | ||
| type Version struct { | ||
| versionString string | ||
| semver semver.Version | ||
| semver *semver.Version | ||
| } | ||
|
|
||
| func New(s string) *Version { | ||
|
|
@@ -37,10 +37,28 @@ func New(s string) *Version { | |
| return v | ||
| } | ||
|
|
||
| func (v *Version) set(s string) error { | ||
| sv, err := semver.NewVersion(s) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| v.versionString = s | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to keep the versionString in memory? Surely
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need it because we want to remember the format of the originally supplied version string. We also don't want a user to submit However, when we infer the Docker image tag, we use the semver, because we don't want docker to simply pull the latest |
||
| v.semver = sv | ||
| return nil | ||
| } | ||
|
|
||
| func (v *Version) Equal(versionB *Version) bool { | ||
| return v.semver.Equal(versionB.semver) | ||
| } | ||
|
|
||
| func (v Version) String() string { | ||
| return v.versionString | ||
| } | ||
|
|
||
| func (v *Version) Semver() *semver.Version { | ||
| return v.semver | ||
| } | ||
|
|
||
| func (v *Version) UnmarshalJSON(data []byte) error { | ||
| s, err := strconv.Unquote(string(data)) | ||
| if err != nil { | ||
|
|
@@ -49,62 +67,20 @@ func (v *Version) UnmarshalJSON(data []byte) error { | |
| return v.set(s) | ||
| } | ||
|
|
||
| func (v *Version) set(cassVersionString string) error { | ||
| var versionsTried []string | ||
| var errorsEncountered []string | ||
|
|
||
| errorWhileParsingOriginalVersion := v.semver.Set(cassVersionString) | ||
| if errorWhileParsingOriginalVersion == nil { | ||
| v.versionString = cassVersionString | ||
| return nil | ||
| } | ||
|
|
||
| versionsTried = append(versionsTried, cassVersionString) | ||
| errorsEncountered = append(errorsEncountered, errorWhileParsingOriginalVersion.Error()) | ||
|
|
||
| semverString := maybeAddMissingPatchVersion(cassVersionString) | ||
| if semverString != cassVersionString { | ||
| errorWhileParsingSemverVersion := v.semver.Set(semverString) | ||
| if errorWhileParsingSemverVersion == nil { | ||
| v.versionString = cassVersionString | ||
| return nil | ||
| } | ||
| versionsTried = append(versionsTried, semverString) | ||
| errorsEncountered = append(errorsEncountered, errorWhileParsingSemverVersion.Error()) | ||
| } | ||
|
|
||
| return fmt.Errorf( | ||
| "unable to parse Cassandra version as semver. "+ | ||
| "Versions tried: '%s'. "+ | ||
| "Errors encountered: '%s'.", | ||
| strings.Join(versionsTried, "','"), | ||
| strings.Join(errorsEncountered, "','"), | ||
| ) | ||
| } | ||
|
|
||
| var _ json.Unmarshaler = &Version{} | ||
|
|
||
| func maybeAddMissingPatchVersion(v string) string { | ||
| mmpAndLabels := strings.SplitN(v, "-", 2) | ||
| mmp := mmpAndLabels[0] | ||
| mmpParts := strings.SplitN(mmp, ".", 3) | ||
| if len(mmpParts) == 2 { | ||
| mmp = mmp + ".0" | ||
| } | ||
| mmpAndLabels[0] = mmp | ||
| return strings.Join(mmpAndLabels, "-") | ||
| } | ||
|
|
||
| func (v Version) String() string { | ||
| return v.versionString | ||
| } | ||
|
|
||
| func (v Version) MarshalJSON() ([]byte, error) { | ||
| return []byte(strconv.Quote(v.String())), nil | ||
| } | ||
|
|
||
| var _ json.Marshaler = &Version{} | ||
|
|
||
| func (v Version) Semver() string { | ||
| return v.semver.String() | ||
| // DeepCopy returns a deep-copy of the Version value. | ||
| // If the underlying semver is a nil pointer, assume that the zero value is being copied, | ||
| // and return that. | ||
| func (v Version) DeepCopy() Version { | ||
| if v.semver == nil { | ||
| return Version{} | ||
| } | ||
| return *New(v.String()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think this needs to be added as a constraint given we are constraining it to master anyway. A simple
dep ensurewithout adding this block should yield the same result, sans this changeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.