-
Notifications
You must be signed in to change notification settings - Fork 27
Add validation to only allow Cassandra major version 3 #321
Changes from all 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 |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
|
|
||
| # Gopkg.toml example | ||
| # | ||
| # Refer to https://github.com/golang/dep/blob/master/docs/Gopkg.toml.md | ||
|
|
||
| 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" | ||
| ) | ||
|
|
||
| // 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
Member
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. 👍 |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ package version_test | |
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/jetstack/navigator/pkg/cassandra/version" | ||
| "github.com/jetstack/navigator/pkg/api/version" | ||
| ) | ||
|
|
||
| func TestUnmarshalJSON(t *testing.T) { | ||
|
|
@@ -21,10 +21,6 @@ func TestUnmarshalJSON(t *testing.T) { | |
| s: `"0.0.x"`, | ||
| expectErr: true, | ||
| }, | ||
| "incomplete semver": { | ||
| s: `"3"`, | ||
| expectErr: true, | ||
| }, | ||
| "cassandra partial invalid semver with labels": { | ||
| s: `"X.Y-foo+bar"`, | ||
| expectErr: true, | ||
|
|
@@ -33,6 +29,12 @@ func TestUnmarshalJSON(t *testing.T) { | |
| s: `"X.Y.0-"`, | ||
| expectErr: true, | ||
| }, | ||
| // Cassandra versions always include a minor version but Hashicorp | ||
| // go-version (which we currently use for parsing) doesn't require it. | ||
| "partial semver": { | ||
| s: `"3"`, | ||
| v: version.New("3.0.0"), | ||
| }, | ||
| "cassandra partial semver": { | ||
| s: `"3.9"`, | ||
| v: version.New("3.9.0"), | ||
|
|
@@ -91,3 +93,18 @@ func TestUnmarshalJSON(t *testing.T) { | |
| ) | ||
| } | ||
| } | ||
|
|
||
| func TestDeepCopy(t *testing.T) { | ||
| t.Run( | ||
| "zero value", | ||
| func(t *testing.T) { | ||
| t.Log(version.Version{}.DeepCopy()) | ||
|
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. This doesn't test anything, or ever fail afaict
Member
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. It tests that a Zero value can be DeepCopied. |
||
| }, | ||
| ) | ||
| t.Run( | ||
| "validated version", | ||
| func(t *testing.T) { | ||
| t.Log(version.New("3.11.2").DeepCopy()) | ||
| }, | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| // +build !ignore_autogenerated | ||
|
|
||
| /* | ||
| Copyright 2018 Jetstack Ltd. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| // This file was autogenerated by deepcopy-gen. Do not edit it manually! | ||
|
|
||
| package version | ||
|
|
||
| // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. | ||
| func (in *Version) DeepCopyInto(out *Version) { | ||
| *out = in.DeepCopy() | ||
| return | ||
| } |
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.
Can we move this file out of
pkg/cassandraand into something likepkg/api/util? This is an API type, but it isn't by default part of an API group (hence it isn't in apis).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)
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.
Done.