Skip to content

Commit

Permalink
fix(local-bom): Fix issues blocking use of locally hosted configurati…
Browse files Browse the repository at this point in the history 
…on files (#1737)

* fix(versions): Fix version parsing for plugin compatibility detection for local BOMs

* fix(versions): Fixing orderBySemVer so it correctly sees equivalent version expressions as equal

* fix(LocalDiskProfileReader): readArchiveProfile should not expect a file extension for profileName, it must supply it

Co-authored-by: Kevin Woo <kevinawoo@gmail.com>
  • Loading branch information
@jaydee864 @kevinawoo
jaydee864 and kevinawoo authored Jul 28, 2020
1 parent ac7f6b3 commit 054fb99
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 29 deletions.
5 changes: 3 additions & 2 deletions .../src/main/java/com/netflix/spinnaker/halyard/core/registry/v1/LocalDiskProfileReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,14 @@ public Versions readVersions() throws IOException {
public InputStream readArchiveProfile(String artifactName, String version, String profileName)
throws IOException {
version = version.substring("local:".length());
String fileName = profileName + ".tar.gz";

try {
Path profilePath = Paths.get(profilePath(artifactName, version, profileName));
Path profilePath = Paths.get(profilePath(artifactName, version, fileName));
return readArchiveProfileFrom(profilePath);
} catch (IOException e) {
log.debug("Failed to get archive profile, retrying default location", e);
Path profilePath = Paths.get(defaultProfilePath(artifactName, profileName));
Path profilePath = Paths.get(defaultProfilePath(artifactName, fileName));
return readArchiveProfileFrom(profilePath);
}
}
Expand Down
28 changes: 21 additions & 7 deletions halyard-core/src/main/java/com/netflix/spinnaker/halyard/core/registry/v1/Versions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,14 @@ private static String truncateToHyphen(String version) {
}

static SemVer fromString(String version) {
String versionExpression = version;
if (isBranch(version)) {
return null;
} else if (isLocal(version)) {
versionExpression = fromLocal(version);
}

String[] parts = truncateToHyphen(version).split(Pattern.quote("."));
String[] parts = truncateToHyphen(versionExpression).split(Pattern.quote("."));
if (parts.length != 3) {
throw new IllegalArgumentException("Versions must satisfy the X.Y.Z naming convention");
}
Expand Down Expand Up @@ -139,27 +142,38 @@ public String toString() {
}

public static String toMajorMinor(String fullVersion) {
int lastDot = fullVersion.lastIndexOf(".");
String version = removePrefixes(fullVersion);
int lastDot = version.lastIndexOf(".");
if (lastDot < 0) {
return null;
}

return fullVersion.substring(0, lastDot);
return version.substring(0, lastDot);
}

public static String toMajorMinorPatch(String fullVersion) {
int lastDash = fullVersion.indexOf("-");
String version = removePrefixes(fullVersion);
int lastDash = version.indexOf("-");
if (lastDash < 0) {
return fullVersion;
return version;
}

return fullVersion.substring(0, lastDash);
return version.substring(0, lastDash);
}

private static String removePrefixes(String fullVersion) {
if (isBranch(fullVersion)) {
return fromBranch(fullVersion);
} else if (isLocal(fullVersion)) {
return fromLocal(fullVersion);
}
return fullVersion;
}

public static Comparator<String> orderBySemVer() {
Comparator<SemVer> comparator = Comparator.nullsLast(SemVer.comparator());
return Comparator.comparing(SemVer::fromString, comparator)
.thenComparing(Comparator.naturalOrder());
.thenComparing(Versions::toMajorMinorPatch, Comparator.naturalOrder());
}

public static boolean lessThan(String v1, String v2) {
Expand Down
60 changes: 42 additions & 18 deletions ...d-core/src/test/groovy/com/netflix/spinnaker/halyard/core/registry/v1/VersionsSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,47 @@ class VersionsSpec extends Specification {
Versions.lessThan(a, b) == result

where:
a | b | result
"1.0.0" | "1.0.0" | false
"1.0.0" | "1.0.1" | true
"1.0.1" | "1.0.0" | false
"2.0.0" | "10.0.0" | true
"1.0.0" | "branch:upstream/master" | true
a | b | result
"1.0.0" | "1.0.0" | false
"1.0.0" | "1.0.1" | true
"1.0.1" | "1.0.0" | false
"2.0.0" | "10.0.0" | true
"1.0.0" | "branch:upstream/master" | true
"local:1.0.0" | "1.0.0" | false
"1.0.0" | "local:1.0.0" | false
"local:1.0.0" | "local:1.0.0" | false
"local:1.0.0" | "1.0.1" | true
"1.0.0" | "local:1.0.1" | true
"local:1.0.0" | "local:1.0.1" | true
"local:1.0.1" | "1.0.0" | false
"1.0.1" | "local:1.0.0" | false
"local:1.0.1" | "local:1.0.0" | false
"local:2.0.0" | "10.0.0" | true
"2.0.0" | "local:10.0.0" | true
"local:2.0.0" | "10.0.0" | true
"local:1.0.0" | "branch:upstream/master" | true
}

def "lessThan looks up to the first hyphen"() {
expect:
Versions.lessThan(a, b) == result

where:
a | b | result
"1.0.0-99999" | "1.0.0-00000" | false
"1.0.0-12345" | "1.0.1-99999" | true
"1.0.0-12345" | "branch:upstream/master" | true
a | b | result
"1.0.0-99999" | "1.0.0-00000" | false
"1.0.0-00000" | "1.0.0-99999" | false
"1.0.0-12345" | "1.0.1-99999" | true
"1.0.0-12345" | "branch:upstream/master" | true
"local:1.0.0-99999" | "1.0.0-00000" | false
"1.0.0-99999" | "local:1.0.0-00000" | false
"local:1.0.0-99999" | "local:1.0.0-00000" | false
"local:1.0.0-00000" | "1.0.0-99999" | false
"1.0.0-00000" | "local:1.0.0-99999" | false
"local:1.0.0-00000" | "local:1.0.0-99999" | false
"local:1.0.0-12345" | "1.0.1-99999" | true
"1.0.0-12345" | "local:1.0.1-99999" | true
"local:1.0.0-12345" | "local:1.0.1-99999" | true
"local:1.0.0-12345" | "branch:upstream/master" | true
}

def "lessThan throws an exception for invalid versions"() {
Expand All @@ -51,34 +75,34 @@ class VersionsSpec extends Specification {
thrown Exception

where:
badVersion << ["1.0", "1.0.0.0", "1.a.b", "zzz"]
badVersion << ["1.0", "1.0.0.0", "1.a.b", "zzz", "local:foo", "local:1.0.0.0"]
}

def "orderBySemVer sorts versions"() {
when:
def versions = ["1.1.0", "2.1.1", "1.1.1"]
def versions = ["1.1.0", "2.1.1", "local:1.1.1"]
Collections.sort(versions, Versions.orderBySemVer());

then:
versions == ["1.1.0", "1.1.1", "2.1.1"]
versions == ["1.1.0", "local:1.1.1", "2.1.1"]
}

def "orderBySemVer sorts each part as an integer"() {
when:
def versions = ["2.2.2", "10.0.0", "0.10.0", "0.0.10"]
def versions = ["2.2.2", "local:10.0.0", "local:0.10.0", "0.0.10"]
Collections.sort(versions, Versions.orderBySemVer())

then:
versions == ["0.0.10", "0.10.0", "2.2.2", "10.0.0"]
versions == ["0.0.10", "local:0.10.0", "2.2.2", "local:10.0.0"]
}

def "orderBySemVer sorts branches to the end"() {
when:
def versions = ["1.0.0", "branch:upstream/master", "1.1.0"]
def versions = ["local:1.0.0", "branch:upstream/master", "1.1.0"]
Collections.sort(versions, Versions.orderBySemVer())

then:
versions == ["1.0.0", "1.1.0", "branch:upstream/master"]
versions == ["local:1.0.0", "1.1.0", "branch:upstream/master"]
}

def "orderBySemVer breaks ties with string sort"() {
Expand All @@ -99,6 +123,6 @@ class VersionsSpec extends Specification {
thrown Exception

where:
badVersion << ["1.0", "1.0.0.0", "1.a.b", "zzz"]
badVersion << ["1.0", "1.0.0.0", "1.a.b", "zzz", "local:foo", "local:1.0.0.0"]
}
}
4 changes: 2 additions & 2 deletions ...t/groovy/com/netflix/spinnaker/halyard/core/resource/v1/LocalDiskProfileReaderSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class LocalDiskProfileReaderSpec extends Specification {
localDiskProfileReader.localBomPath = new File(".").getCanonicalPath() + "/src/test/resources/profiles"
String artifactName = "test-artifact"
String version = "local:test-version"
String profileName = "test-artifact.tar.gz"
String profileName = "test-artifact"
when:
def archiveProfileStream = localDiskProfileReader.readArchiveProfile(artifactName, version, profileName)
TarArchiveInputStream tis
Expand All @@ -105,7 +105,7 @@ class LocalDiskProfileReaderSpec extends Specification {
} catch (IOException e) {
throw new HalException(Problem.Severity.FATAL, "Failed to read profile entry", e)
}
def fileInputStream = localDiskProfileReader.readProfile(artifactName, version, profileName)
def fileInputStream = localDiskProfileReader.readProfile(artifactName, version, profileName + ".tar.gz")
def fileContents = IOUtils.toString(fileInputStream)
then:
tarContents == fileContents
Expand Down
5 changes: 5 additions & 0 deletions ...com/netflix/spinnaker/halyard/deploy/spinnaker/v1/profile/SpringProfileFactorySpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,10 @@ class SpringProfileFactorySpec extends Specification {
"release-1.19.x-20200503040016" | true
"release-1.19.x-latest-validated" | true
"release-1.7.x-latest-validated" | false
"local:1.7.5" | false
"local:1.19.3" | false
"local:1.19.11" | true
"branch:master-20191121162350" | false
"branch:master-20200503040016" | false
}
}

0 comments on commit 054fb99

Please sign in to comment.