diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 621e2b518..ebf093d3f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: actions/setup-java@v3 + - uses: actions/setup-java@v4 with: java-version: 11 distribution: 'zulu' diff --git a/.github/workflows/gradle-wrapper-validation.yml b/.github/workflows/gradle-wrapper-validation.yml index a8195158c..f5ccf04c7 100644 --- a/.github/workflows/gradle-wrapper-validation.yml +++ b/.github/workflows/gradle-wrapper-validation.yml @@ -7,4 +7,4 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: gradle/wrapper-validation-action@v1 + - uses: gradle/wrapper-validation-action@v2 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 54b60a91e..3d4281aa3 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: actions/setup-java@v3 + - uses: actions/setup-java@v4 with: java-version: 11 distribution: 'zulu' diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0ca6be4b6..bc3d15a34 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -35,7 +35,7 @@ jobs: fi echo "exactly one branch ($BRANCHES)" echo BRANCH=$BRANCHES >> $GITHUB_ENV - - uses: actions/setup-java@v3 + - uses: actions/setup-java@v4 with: java-version: 11 distribution: 'zulu' @@ -74,7 +74,7 @@ jobs: - name: Pause before dependency bump run: sleep 600 - name: Trigger dependency bump workflow - uses: peter-evans/repository-dispatch@v2 + uses: peter-evans/repository-dispatch@v3 with: token: ${{ secrets.SPINNAKER_GITHUB_TOKEN }} event-type: bump-dependencies diff --git a/build.gradle b/build.gradle index 39c639391..045fb39d3 100644 --- a/build.gradle +++ b/build.gradle @@ -31,14 +31,12 @@ subprojects { if (it.name != "kork-bom" && it.name != "spinnaker-dependencies") { apply plugin: 'java-library' test { - useJUnitPlatform { - includeEngines "spek2", "junit-vintage", "junit-jupiter" - } + useJUnitPlatform() } dependencies { annotationProcessor(platform(project(":spinnaker-dependencies"))) annotationProcessor("org.springframework.boot:spring-boot-configuration-processor") - testRuntimeOnly("org.junit.vintage:junit-vintage-engine") + testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine" } } } diff --git a/gradle.properties b/gradle.properties index 8aab42b43..0c2557f59 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,5 +1,5 @@ -kotlinVersion=1.4.32 +kotlinVersion=1.5.32 org.gradle.parallel=true -spinnakerGradleVersion=8.31.0 +spinnakerGradleVersion=8.32.1 targetJava11=true includeRuntimes=actuator,core,eureka,retrofit,secrets-aws,secrets-gcp,stackdriver,swagger,tomcat,web diff --git a/gradle/kotlin-test.gradle b/gradle/kotlin-test.gradle index b8a2f8229..b6d4c5021 100644 --- a/gradle/kotlin-test.gradle +++ b/gradle/kotlin-test.gradle @@ -20,6 +20,7 @@ apply plugin: "kotlin" dependencies { + testImplementation(platform(project(":spinnaker-dependencies"))) testImplementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlinVersion" testImplementation "org.junit.jupiter:junit-jupiter-api" @@ -29,13 +30,14 @@ dependencies { testImplementation "dev.minutest:minutest" testImplementation "io.mockk:mockk" + testRuntimeOnly(platform(project(":spinnaker-dependencies"))) testRuntimeOnly "org.junit.platform:junit-platform-launcher" testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine" } compileTestKotlin { kotlinOptions { - languageVersion = "1.4" + languageVersion = "1.5" jvmTarget = "11" } } diff --git a/gradle/kotlin.gradle b/gradle/kotlin.gradle index e2884cf25..203c24938 100644 --- a/gradle/kotlin.gradle +++ b/gradle/kotlin.gradle @@ -19,6 +19,7 @@ apply plugin: "kotlin-spring" apply plugin: "io.gitlab.arturbosch.detekt" dependencies { + testImplementation(platform(project(":spinnaker-dependencies"))) testImplementation "org.junit.jupiter:junit-jupiter-api" testImplementation "org.junit.platform:junit-platform-runner" testImplementation "org.spekframework.spek2:spek-dsl-jvm" @@ -28,6 +29,7 @@ dependencies { testImplementation "dev.minutest:minutest" testImplementation "io.mockk:mockk" + testRuntimeOnly(platform(project(":spinnaker-dependencies"))) testRuntimeOnly "org.junit.platform:junit-platform-launcher" testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine" testRuntimeOnly "org.junit.vintage:junit-vintage-engine" @@ -37,14 +39,14 @@ dependencies { compileKotlin { kotlinOptions { - languageVersion = "1.4" + languageVersion = "1.5" jvmTarget = "11" } } compileTestKotlin { kotlinOptions { - languageVersion = "1.4" + languageVersion = "1.5" jvmTarget = "11" } } diff --git a/gradle/lombok.gradle b/gradle/lombok.gradle index a94257fc1..93f41b25d 100644 --- a/gradle/lombok.gradle +++ b/gradle/lombok.gradle @@ -1,8 +1,10 @@ dependencies { compileOnly "org.projectlombok:lombok" + compileOnly(platform(project(":spinnaker-dependencies"))) annotationProcessor "org.projectlombok:lombok" annotationProcessor(platform(project(":spinnaker-dependencies"))) testCompileOnly "org.projectlombok:lombok" + testCompileOnly(platform(project(":spinnaker-dependencies"))) testAnnotationProcessor(platform(project(":spinnaker-dependencies"))) testAnnotationProcessor "org.projectlombok:lombok" } diff --git a/kork-actuator/kork-actuator.gradle b/kork-actuator/kork-actuator.gradle index 13686dfa7..0f441b752 100644 --- a/kork-actuator/kork-actuator.gradle +++ b/kork-actuator/kork-actuator.gradle @@ -19,6 +19,7 @@ apply plugin: "java-library" apply from: "$rootDir/gradle/kotlin-test.gradle" dependencies { + compileOnly(platform(project(":spinnaker-dependencies"))) implementation(platform(project(":spinnaker-dependencies"))) implementation "org.springframework.security:spring-security-core" implementation "org.springframework.boot:spring-boot-starter-security" diff --git a/kork-artifacts/kork-artifacts.gradle b/kork-artifacts/kork-artifacts.gradle index 6d1b75509..a6a8b0fe9 100644 --- a/kork-artifacts/kork-artifacts.gradle +++ b/kork-artifacts/kork-artifacts.gradle @@ -20,6 +20,7 @@ dependencies { testImplementation "org.junit.jupiter:junit-jupiter-api" testImplementation "org.junit.jupiter:junit-jupiter-params" testImplementation "org.mockito:mockito-core" + testImplementation "org.springframework.boot:spring-boot-starter-test" testImplementation project(":kork-core") testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine" } diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/README.md b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/README.md index 39433b123..0308f4093 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/README.md +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/README.md @@ -81,12 +81,25 @@ To enable artifact storage, simple add this to your `spinnaker-local.yml` file ```yaml artifact-store: - enabled: true + type: s3 s3: enabled: true bucket: some-artifact-store-bucket ``` +### Rosco and Helm + +If any pipelines are passing artifact references to bake stages as a parameter, +enabling this field will allow those URIs to be expanded to the full +references: + +```yaml +artifact-store: + type: s3 + helm: + expandOverrides: true +``` + ## Storage Options ### S3 @@ -97,7 +110,7 @@ against AWS. ```yaml artifact-store: - enabled: true + type: s3 s3: enabled: true profile: dev # if you want to authenticate using a certain profile @@ -119,7 +132,7 @@ Next enable the configuration ```yaml artifact-store: - enabled: true + type: s3 s3: enabled: true url: http://localhost:8333 # this URL will be used to make S3 API requests to diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactReferenceURI.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactReferenceURI.java index 8f1cbb022..3fdfcfa0e 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactReferenceURI.java +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactReferenceURI.java @@ -29,24 +29,31 @@ @Builder @Getter public class ArtifactReferenceURI { - private final String scheme; + /** + * uriScheme is used as an HTTP scheme to let us further distinguish a String that is a URI to an + * artifact. This is helpful in determining what is an artifact since sometimes we are only given + * a string rather than a full artifact. + */ + private static final String uriScheme = "ref://"; + private final List uriPaths; public String uri() { - return String.format("%s://%s", scheme, paths()); + return uriScheme + paths(); } public String paths() { return Strings.join(uriPaths, '/'); } + /** Used to determine whether a String is in the artifact reference URI format. */ + public static boolean is(String reference) { + return reference.startsWith(uriScheme); + } + public static ArtifactReferenceURI parse(String reference) { - String noSchemeURI = - StringUtils.removeStart(reference, ArtifactStoreURIBuilder.uriScheme + "://"); + String noSchemeURI = StringUtils.removeStart(reference, uriScheme); String[] paths = StringUtils.split(noSchemeURI, '/'); - return ArtifactReferenceURI.builder() - .scheme(ArtifactStoreURIBuilder.uriScheme) - .uriPaths(Arrays.asList(paths)) - .build(); + return ArtifactReferenceURI.builder().uriPaths(Arrays.asList(paths)).build(); } } diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStore.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStore.java index 65b1dc62e..7e2d9566e 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStore.java +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStore.java @@ -16,35 +16,45 @@ package com.netflix.spinnaker.kork.artifacts.artifactstore; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import java.util.concurrent.atomic.AtomicBoolean; import lombok.Getter; +import lombok.extern.log4j.Log4j2; -/** - * ArtifactStore is an interface that allows for different types of artifact storage to be used - * during runtime - */ -public abstract class ArtifactStore { - /** ensures the singleton has only been set once */ - private static final AtomicBoolean singletonSet = new AtomicBoolean(false); +/** ArtifactStore allows for different types of artifact storage to be used during runtime */ +@Log4j2 +public class ArtifactStore implements ArtifactStoreGetter, ArtifactStoreStorer { + @Getter private static volatile ArtifactStore instance = null; + + private final ArtifactStoreGetter artifactStoreGetter; + + private final ArtifactStoreStorer artifactStoreStorer; - @Getter private static ArtifactStore instance = null; + public ArtifactStore( + ArtifactStoreGetter artifactStoreGetter, ArtifactStoreStorer artifactStoreStorer) { + this.artifactStoreGetter = artifactStoreGetter; + this.artifactStoreStorer = artifactStoreStorer; + } + + /** Store an artifact in the artifact store */ + public Artifact store(Artifact artifact) { + return artifactStoreStorer.store(artifact); + } - public abstract Artifact store(Artifact artifact); /** * get is used to return an artifact with some id, while also decorating that artifact with any * necessary fields needed which should be then be returned by the artifact store. */ - public abstract Artifact get(ArtifactReferenceURI uri, ArtifactDecorator... decorators); + public Artifact get(ArtifactReferenceURI uri, ArtifactDecorator... decorators) { + return artifactStoreGetter.get(uri, decorators); + } public static void setInstance(ArtifactStore storage) { - if (!singletonSet.compareAndSet(false, true)) { - throw new IllegalStateException("Multiple attempts at setting ArtifactStore's singleton"); - } + synchronized (ArtifactStore.class) { + if (instance == null) { + instance = storage; + return; + } - ArtifactStore.instance = storage; - } - - public boolean isArtifactURI(String value) { - return value.startsWith(ArtifactStoreURIBuilder.uriScheme + "://"); + log.warn("Multiple attempts in setting the singleton artifact store"); + } } } diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfiguration.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfiguration.java index 0b122384a..c2a0b6d60 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfiguration.java +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfiguration.java @@ -16,29 +16,16 @@ package com.netflix.spinnaker.kork.artifacts.artifactstore; import com.fasterxml.jackson.databind.ObjectMapper; -import com.netflix.spinnaker.kork.artifacts.artifactstore.s3.S3ArtifactStore; -import java.net.URI; -import java.util.Optional; -import lombok.extern.log4j.Log4j2; -import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; +import com.netflix.spinnaker.kork.artifacts.artifactstore.s3.S3ArtifactStoreConfiguration; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.security.access.PermissionEvaluator; -import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider; -import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; -import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; -import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider; -import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; -import software.amazon.awssdk.regions.Region; -import software.amazon.awssdk.services.s3.S3Client; -import software.amazon.awssdk.services.s3.S3ClientBuilder; +import org.springframework.context.annotation.Import; @Configuration -@ConditionalOnExpression("${artifact-store.enabled:false}") @EnableConfigurationProperties(ArtifactStoreConfigurationProperties.class) -@Log4j2 +@Import(S3ArtifactStoreConfiguration.class) public class ArtifactStoreConfiguration { /** * this is strictly used due to Spring and Jackson not behaving nicely together. @@ -55,63 +42,23 @@ public ArtifactStoreURIBuilder artifactStoreURIBuilder() { return new ArtifactStoreURISHA256Builder(); } + @ConditionalOnMissingBean(ArtifactStoreGetter.class) @Bean - @ConditionalOnExpression("${artifact-store.s3.enabled:false}") - public ArtifactStore s3ArtifactStore( - Optional permissionEvaluator, - ArtifactStoreConfigurationProperties properties, - @Qualifier("artifactS3Client") S3Client s3Client, - ArtifactStoreURIBuilder artifactStoreURIBuilder) { - - if (permissionEvaluator.isEmpty()) { - log.warn( - "PermissionEvaluator is not present. This means anyone will be able to access any artifact in the store."); - } - - ArtifactStore storage = - new S3ArtifactStore( - s3Client, - permissionEvaluator.orElse(null), - properties.getS3().getBucket(), - artifactStoreURIBuilder, - properties.getApplicationsRegex()); - - ArtifactStore.setInstance(storage); - return storage; + public ArtifactStoreGetter artifactStoreGetter() { + return new NoopArtifactStoreGetter(); } + @ConditionalOnMissingBean(ArtifactStoreStorer.class) @Bean - @ConditionalOnExpression("${artifact-store.s3.enabled:false}") - public S3Client artifactS3Client(ArtifactStoreConfigurationProperties properties) { - S3ClientBuilder builder = S3Client.builder(); - ArtifactStoreConfigurationProperties.S3ClientConfig config = properties.getS3(); - - // Overwriting the URL is primarily used for S3 compatible object stores - // like seaweedfs - if (config.getUrl() != null) { - builder = - builder - .credentialsProvider(getCredentialsProvider(config)) - .forcePathStyle(config.isForcePathStyle()) - .endpointOverride(URI.create(config.getUrl())); - } else if (config.getProfile() != null) { - builder = builder.credentialsProvider(ProfileCredentialsProvider.create(config.getProfile())); - } - - if (config.getRegion() != null) { - builder = builder.region(Region.of(config.getRegion())); - } - - return builder.build(); + public ArtifactStoreStorer artifactStoreStorer() { + return new NoopArtifactStoreStorer(); } - private AwsCredentialsProvider getCredentialsProvider( - ArtifactStoreConfigurationProperties.S3ClientConfig config) { - if (config.getAccessKey() != null) { - return StaticCredentialsProvider.create( - AwsBasicCredentials.create(config.getAccessKey(), config.getSecretKey())); - } else { - return AnonymousCredentialsProvider.create(); - } + @Bean + public ArtifactStore artifactStore( + ArtifactStoreGetter artifactStoreGetter, ArtifactStoreStorer artifactStoreStorer) { + ArtifactStore artifactStore = new ArtifactStore(artifactStoreGetter, artifactStoreStorer); + ArtifactStore.setInstance(artifactStore); + return artifactStore; } } diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfigurationProperties.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfigurationProperties.java index e3fbf51be..6f06ae786 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfigurationProperties.java +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfigurationProperties.java @@ -22,6 +22,10 @@ @ConfigurationProperties("artifact-store") public class ArtifactStoreConfigurationProperties { private String applicationsRegex = null; + + /** The type of artifact store to use (e.g. s3). */ + private String type = null; + /** Configuration for an s3 client which will utilize credentials in the AWS credentials file. */ @Data public static class S3ClientConfig { @@ -40,5 +44,12 @@ public static class S3ClientConfig { private boolean forcePathStyle = true; } - private S3ClientConfig s3 = null; + @Data + public static class HelmConfig { + /** Enables Rosco to expand any artifact URIs passed as parameters for Helm. */ + private boolean expandOverrides = false; + } + + private S3ClientConfig s3 = new S3ClientConfig(); + private HelmConfig helm = new HelmConfig(); } diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreGetter.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreGetter.java new file mode 100644 index 000000000..3a4fd6cef --- /dev/null +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreGetter.java @@ -0,0 +1,30 @@ +/* + * Copyright 2023 Salesforce Inc. + * + * 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. + */ +package com.netflix.spinnaker.kork.artifacts.artifactstore; + +import com.netflix.spinnaker.kork.artifacts.model.Artifact; + +/** + * ArtifactStoreGetter is an interface that allows for different types of artifact storage to be + * used during runtime. + */ +public interface ArtifactStoreGetter { + /** + * get is used to return an artifact with some id, while also decorating that artifact with any + * necessary fields needed which should be then be returned by the artifact store. + */ + public Artifact get(ArtifactReferenceURI uri, ArtifactDecorator... decorators); +} diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreStorer.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreStorer.java new file mode 100644 index 000000000..d817ac351 --- /dev/null +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreStorer.java @@ -0,0 +1,27 @@ +/* + * Copyright 2023 Salesforce Inc. + * + * 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. + */ +package com.netflix.spinnaker.kork.artifacts.artifactstore; + +import com.netflix.spinnaker.kork.artifacts.model.Artifact; + +/** + * ArtifactStoreStorer is an interface that allows for different types of artifact storage to be + * used during runtime. + */ +public interface ArtifactStoreStorer { + + Artifact store(Artifact artifact); +} diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreURIBuilder.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreURIBuilder.java index ba6712008..cef9904e4 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreURIBuilder.java +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreURIBuilder.java @@ -18,13 +18,6 @@ import com.netflix.spinnaker.kork.artifacts.model.Artifact; public abstract class ArtifactStoreURIBuilder { - /** - * uriScheme is used as an HTTP scheme to let us further distinguish a String that is a URI to an - * artifact. This is helpful in determining what is an artifact since sometimes we are only given - * a string rather than a full artifact. - */ - public static final String uriScheme = "ref"; - /** * Returns the remote artifact URI that will be associated with some artifact. * diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreURISHA256Builder.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreURISHA256Builder.java index b7744515b..ee739eaa0 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreURISHA256Builder.java +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreURISHA256Builder.java @@ -38,7 +38,7 @@ public ArtifactReferenceURI buildArtifactURI(String context, Artifact artifact) Hashing.sha256() .hashBytes(artifact.getReference().getBytes(StandardCharsets.UTF_8)) .toString()); - return ArtifactReferenceURI.builder().scheme(uriScheme).uriPaths(uriPaths).build(); + return ArtifactReferenceURI.builder().uriPaths(uriPaths).build(); } @Override @@ -47,6 +47,6 @@ public ArtifactReferenceURI buildURIFromPaths(String context, String... paths) { uriPaths.add(context); uriPaths.addAll(List.of(paths)); - return ArtifactReferenceURI.builder().scheme(uriScheme).uriPaths(uriPaths).build(); + return ArtifactReferenceURI.builder().uriPaths(uriPaths).build(); } } diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/NoopArtifactStoreGetter.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/NoopArtifactStoreGetter.java new file mode 100644 index 000000000..e1b8dbeec --- /dev/null +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/NoopArtifactStoreGetter.java @@ -0,0 +1,29 @@ +/* + * Copyright 2023 Salesforce Inc. + * + * 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. + */ +package com.netflix.spinnaker.kork.artifacts.artifactstore; + +import com.netflix.spinnaker.kork.artifacts.model.Artifact; + +/** A no-op ArtifactStoreGetter. In other words, don't actually get the artifact. */ +public class NoopArtifactStoreGetter implements ArtifactStoreGetter { + + public Artifact get(ArtifactReferenceURI uri, ArtifactDecorator... decorators) { + throw new IllegalStateException( + "unable to retrieve artifact " + + uri.toString() + + " since there's no artifact store getter configured"); + } +} diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/NoopArtifactStoreStorer.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/NoopArtifactStoreStorer.java new file mode 100644 index 000000000..b441dfd9d --- /dev/null +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/NoopArtifactStoreStorer.java @@ -0,0 +1,26 @@ +/* + * Copyright 2023 Salesforce Inc. + * + * 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. + */ +package com.netflix.spinnaker.kork.artifacts.artifactstore; + +import com.netflix.spinnaker.kork.artifacts.model.Artifact; + +/** A no-op ArtifactStoreStorer. In other words, don't actually store the artifact. */ +public class NoopArtifactStoreStorer implements ArtifactStoreStorer { + + public Artifact store(Artifact artifact) { + return artifact; + } +} diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStore.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStore.java index 14e807d09..a73b2fbe2 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStore.java +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStore.java @@ -15,233 +15,8 @@ */ package com.netflix.spinnaker.kork.artifacts.artifactstore.s3; -import com.netflix.spinnaker.kork.artifacts.ArtifactTypes; -import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactDecorator; -import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactReferenceURI; -import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStore; -import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreURIBuilder; -import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import com.netflix.spinnaker.kork.exceptions.SpinnakerException; -import com.netflix.spinnaker.security.AuthenticatedRequest; -import java.util.Base64; -import java.util.NoSuchElementException; -import java.util.regex.Pattern; -import lombok.extern.log4j.Log4j2; -import org.apache.http.HttpStatus; -import org.springframework.security.access.PermissionEvaluator; -import org.springframework.security.authentication.AuthenticationServiceException; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; -import software.amazon.awssdk.core.ResponseBytes; -import software.amazon.awssdk.core.sync.RequestBody; -import software.amazon.awssdk.services.s3.S3Client; -import software.amazon.awssdk.services.s3.model.GetObjectRequest; -import software.amazon.awssdk.services.s3.model.GetObjectResponse; -import software.amazon.awssdk.services.s3.model.GetObjectTaggingRequest; -import software.amazon.awssdk.services.s3.model.GetObjectTaggingResponse; -import software.amazon.awssdk.services.s3.model.HeadObjectRequest; -import software.amazon.awssdk.services.s3.model.NoSuchKeyException; -import software.amazon.awssdk.services.s3.model.PutObjectRequest; -import software.amazon.awssdk.services.s3.model.S3Exception; -import software.amazon.awssdk.services.s3.model.Tag; -import software.amazon.awssdk.services.s3.model.Tagging; +public final class S3ArtifactStore { + public static final String ENFORCE_PERMS_KEY = "application"; -/** - * S3ArtifactStore will store artifacts in a s3 compatible service - * - *

Note: It is very important that the S3 bucket has object lock on it to prevent multiple writes - * {@see https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-lock-overview.html} - */ -@Log4j2 -public class S3ArtifactStore extends ArtifactStore { - private final S3Client s3Client; - private final PermissionEvaluator permissionEvaluator; - private final String bucket; - private final ArtifactStoreURIBuilder uriBuilder; - private final String applicationsRegex; - private static final String ENFORCE_PERMS_KEY = "application"; - - public S3ArtifactStore( - S3Client s3Client, - PermissionEvaluator permissionEvaluator, - String bucket, - ArtifactStoreURIBuilder uriBuilder, - String applicationsRegex) { - this.s3Client = s3Client; - this.bucket = bucket; - this.permissionEvaluator = permissionEvaluator; - this.uriBuilder = uriBuilder; - this.applicationsRegex = applicationsRegex; - } - - /** - * Will store the artifact using the {@link #s3Client} in some {@link #bucket} - * - *

This method also persists "permissions" by storing the execution id that made the original - * store call. In the event a service wants to retrieve said artifact, they will also need to - * provide the proper execution id - */ - @Override - public Artifact store(Artifact artifact) { - String application = AuthenticatedRequest.getSpinnakerApplication().orElse(null); - if (application == null) { - log.warn("failed to retrieve application from request artifact={}", artifact.getName()); - return artifact; - } - - if (applicationsRegex != null && !Pattern.matches(applicationsRegex, application)) { - return artifact; - } - - ArtifactReferenceURI ref = uriBuilder.buildArtifactURI(application, artifact); - Artifact remoteArtifact = - artifact.toBuilder() - .type(ArtifactTypes.REMOTE_BASE64.getMimeType()) - .reference(ref.uri()) - .build(); - - if (objectExists(ref)) { - return remoteArtifact; - } - - // purpose of tagging is to ensure some sort of identity is persisted to - // enforce permissions when retrieving the artifact - Tag accountTag = Tag.builder().key(ENFORCE_PERMS_KEY).value(application).build(); - - PutObjectRequest request = - PutObjectRequest.builder() - .bucket(bucket) - .key(ref.paths()) - .tagging(Tagging.builder().tagSet(accountTag).build()) - .build(); - - s3Client.putObject(request, RequestBody.fromBytes(getReferenceAsBytes(artifact))); - return remoteArtifact; - } - - private byte[] getReferenceAsBytes(Artifact artifact) { - String reference = artifact.getReference(); - if (reference == null) { - throw new IllegalArgumentException("reference cannot be null"); - } - - String type = artifact.getType(); - if (type != null && type.endsWith("/base64")) { - return Base64.getDecoder().decode(reference); - } - - return reference.getBytes(); - } - - /** - * get will return the Artifact with the provided id, and will lastly run the {@link - * ArtifactDecorator} to further populate the artifact for returning - */ - @Override - public Artifact get(ArtifactReferenceURI uri, ArtifactDecorator... decorators) { - hasAuthorization( - uri, - AuthenticatedRequest.getSpinnakerUser() - .orElseThrow( - () -> new NoSuchElementException("Could not authenticate due to missing user id"))); - - GetObjectRequest request = GetObjectRequest.builder().bucket(bucket).key(uri.paths()).build(); - - ResponseBytes resp = s3Client.getObjectAsBytes(request); - Artifact.ArtifactBuilder builder = - Artifact.builder() - .type(ArtifactTypes.REMOTE_BASE64.getMimeType()) - .reference(Base64.getEncoder().encodeToString(resp.asByteArray())); - - if (decorators == null) { - return builder.build(); - } - - for (ArtifactDecorator decorator : decorators) { - builder = decorator.decorate(builder); - } - - return builder.build(); - } - - /** - * hasAuthorization will ensure that the user has proper permissions for retrieving the stored - * artifact - * - * @throws AuthenticationServiceException when user does not have correct permissions - */ - private void hasAuthorization(ArtifactReferenceURI uri, String userId) { - GetObjectTaggingRequest request = - GetObjectTaggingRequest.builder().bucket(bucket).key(uri.paths()).build(); - - GetObjectTaggingResponse resp = s3Client.getObjectTagging(request); - Tag tag = - resp.tagSet().stream() - .filter(t -> t.key().equals(ENFORCE_PERMS_KEY)) - .findFirst() - .orElse(null); - Authentication auth = SecurityContextHolder.getContext().getAuthentication(); - - if (tag == null - || (permissionEvaluator != null - && !permissionEvaluator.hasPermission(auth, tag.value(), "application", "READ"))) { - log.error( - "Could not authenticate to retrieve artifact user={} applicationOfStoredArtifact={}", - userId, - (tag == null) ? "(none)" : tag.value()); - throw new AuthenticationServiceException( - userId + " does not have permission to access this artifact"); - } - } - - /** - * Helper method to check whether the object exists. This is not thread safe, nor would it help in - * a distributed system due to how S3 works (no conditional statements). If preventing multiple - * writes of the same object is important, another filestore/db needs to be used, possibly - * dynamodb. - */ - private boolean objectExists(ArtifactReferenceURI uri) { - HeadObjectRequest request = HeadObjectRequest.builder().bucket(bucket).key(uri.paths()).build(); - try { - s3Client.headObject(request); - log.debug("Artifact exists. No need to store. reference={}", uri.uri()); - return true; - } catch (NoSuchKeyException e) { - // pretty gross that we need to use exceptions as control flow, but the - // java SDK doesn't have any other way of check if an object exists in s3 - log.info("Artifact does not exist reference={}", uri.uri()); - return false; - } catch (S3Exception e) { - int statusCode = e.statusCode(); - log.error( - "Artifact store failed head object request statusCode={} reference={}", - statusCode, - uri.uri()); - - if (statusCode != 0) { - // due to this being a HEAD request, there is no message giving a clear - // indication of what failed. Rather than seeing a useful message back - // to gate, we instead see just null. To alleviate this, we wrap the - // exception with a more meaningful message - throw new SpinnakerException(buildHeadObjectExceptionMessage(e), e); - } - - throw new SpinnakerException("S3 head object failed", e); - } - } - - /** - * S3's head object can only return 400, 403, and 404, and based on the HTTP status code, we will - * return the appropriate message back - */ - private static String buildHeadObjectExceptionMessage(S3Exception e) { - switch (e.statusCode()) { - case HttpStatus.SC_FORBIDDEN: - return "Failed to query artifact due to IAM permissions either on the bucket or object"; - case HttpStatus.SC_BAD_REQUEST: - return "Failed to query artifact due to invalid request"; - default: - return String.format("Failed to query artifact: %d", e.statusCode()); - } - } + private S3ArtifactStore() {} } diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreConfiguration.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreConfiguration.java new file mode 100644 index 000000000..e792d0c13 --- /dev/null +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreConfiguration.java @@ -0,0 +1,107 @@ +/* + * Copyright 2023 Apple Inc. + * + * 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. + */ +package com.netflix.spinnaker.kork.artifacts.artifactstore.s3; + +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreConfigurationProperties; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreGetter; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreStorer; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreURIBuilder; +import java.net.URI; +import java.util.Optional; +import lombok.extern.log4j.Log4j2; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.access.PermissionEvaluator; +import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; +import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider; +import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.S3ClientBuilder; + +@Configuration +@Log4j2 +@ConditionalOnProperty(name = "artifact-store.type", havingValue = "s3") +public class S3ArtifactStoreConfiguration { + + @Bean + @ConditionalOnExpression("${artifact-store.s3.enabled:false}") + public ArtifactStoreStorer artifactStoreStorer( + ArtifactStoreConfigurationProperties properties, + @Qualifier("artifactS3Client") S3Client s3Client, + ArtifactStoreURIBuilder artifactStoreURIBuilder) { + return new S3ArtifactStoreStorer( + s3Client, + properties.getS3().getBucket(), + artifactStoreURIBuilder, + properties.getApplicationsRegex()); + } + + @Bean + public ArtifactStoreGetter artifactStoreGetter( + Optional permissionEvaluator, + ArtifactStoreConfigurationProperties properties, + @Qualifier("artifactS3Client") S3Client s3Client) { + + if (permissionEvaluator.isEmpty()) { + log.warn( + "PermissionEvaluator is not present. This means anyone will be able to access any artifact in the store."); + } + + String bucket = properties.getS3().getBucket(); + + return new S3ArtifactStoreGetter(s3Client, permissionEvaluator.orElse(null), bucket); + } + + @Bean + public S3Client artifactS3Client(ArtifactStoreConfigurationProperties properties) { + S3ClientBuilder builder = S3Client.builder(); + ArtifactStoreConfigurationProperties.S3ClientConfig config = properties.getS3(); + + // Overwriting the URL is primarily used for S3 compatible object stores + // like seaweedfs + if (config.getUrl() != null) { + builder = + builder + .credentialsProvider(getCredentialsProvider(config)) + .forcePathStyle(config.isForcePathStyle()) + .endpointOverride(URI.create(config.getUrl())); + } else if (config.getProfile() != null) { + builder = builder.credentialsProvider(ProfileCredentialsProvider.create(config.getProfile())); + } + + if (config.getRegion() != null) { + builder = builder.region(Region.of(config.getRegion())); + } + + return builder.build(); + } + + private AwsCredentialsProvider getCredentialsProvider( + ArtifactStoreConfigurationProperties.S3ClientConfig config) { + if (config.getAccessKey() != null) { + return StaticCredentialsProvider.create( + AwsBasicCredentials.create(config.getAccessKey(), config.getSecretKey())); + } else { + return AnonymousCredentialsProvider.create(); + } + } +} diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetter.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetter.java new file mode 100644 index 000000000..778c46528 --- /dev/null +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreGetter.java @@ -0,0 +1,115 @@ +/* + * Copyright 2023 Apple Inc. + * + * 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. + */ +package com.netflix.spinnaker.kork.artifacts.artifactstore.s3; + +import static com.netflix.spinnaker.kork.artifacts.artifactstore.s3.S3ArtifactStore.ENFORCE_PERMS_KEY; + +import com.netflix.spinnaker.kork.artifacts.ArtifactTypes; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactDecorator; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactReferenceURI; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreGetter; +import com.netflix.spinnaker.kork.artifacts.model.Artifact; +import com.netflix.spinnaker.security.AuthenticatedRequest; +import java.util.Base64; +import java.util.NoSuchElementException; +import lombok.extern.log4j.Log4j2; +import org.springframework.security.access.PermissionEvaluator; +import org.springframework.security.authentication.AuthenticationServiceException; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import software.amazon.awssdk.core.ResponseBytes; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.GetObjectRequest; +import software.amazon.awssdk.services.s3.model.GetObjectResponse; +import software.amazon.awssdk.services.s3.model.GetObjectTaggingRequest; +import software.amazon.awssdk.services.s3.model.GetObjectTaggingResponse; +import software.amazon.awssdk.services.s3.model.Tag; + +/** Retrieve objects from an s3 compatible service */ +@Log4j2 +public class S3ArtifactStoreGetter implements ArtifactStoreGetter { + private final S3Client s3Client; + private final PermissionEvaluator permissionEvaluator; + private final String bucket; + + public S3ArtifactStoreGetter( + S3Client s3Client, PermissionEvaluator permissionEvaluator, String bucket) { + this.s3Client = s3Client; + this.bucket = bucket; + this.permissionEvaluator = permissionEvaluator; + } + + /** + * get will return the Artifact with the provided id, and will lastly run the {@link + * ArtifactDecorator} to further populate the artifact for returning + */ + @Override + public Artifact get(ArtifactReferenceURI uri, ArtifactDecorator... decorators) { + hasAuthorization( + uri, + AuthenticatedRequest.getSpinnakerUser() + .orElseThrow( + () -> new NoSuchElementException("Could not authenticate due to missing user id"))); + + GetObjectRequest request = GetObjectRequest.builder().bucket(bucket).key(uri.paths()).build(); + + ResponseBytes resp = s3Client.getObjectAsBytes(request); + Artifact.ArtifactBuilder builder = + Artifact.builder() + .type(ArtifactTypes.REMOTE_BASE64.getMimeType()) + .reference(Base64.getEncoder().encodeToString(resp.asByteArray())); + + if (decorators == null) { + return builder.build(); + } + + for (ArtifactDecorator decorator : decorators) { + builder = decorator.decorate(builder); + } + + return builder.build(); + } + + /** + * hasAuthorization will ensure that the user has proper permissions for retrieving the stored + * artifact + * + * @throws AuthenticationServiceException when user does not have correct permissions + */ + private void hasAuthorization(ArtifactReferenceURI uri, String userId) { + GetObjectTaggingRequest request = + GetObjectTaggingRequest.builder().bucket(bucket).key(uri.paths()).build(); + + GetObjectTaggingResponse resp = s3Client.getObjectTagging(request); + Tag tag = + resp.tagSet().stream() + .filter(t -> t.key().equals(ENFORCE_PERMS_KEY)) + .findFirst() + .orElse(null); + Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + + if (tag == null + || (permissionEvaluator != null + && !permissionEvaluator.hasPermission(auth, tag.value(), "application", "READ"))) { + log.error( + "Could not authenticate to retrieve artifact user={} applicationOfStoredArtifact={}", + userId, + (tag == null) ? "(none)" : tag.value()); + throw new AuthenticationServiceException( + userId + " does not have permission to access this artifact"); + } + } +} diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreStorer.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreStorer.java new file mode 100644 index 000000000..bb767e398 --- /dev/null +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreStorer.java @@ -0,0 +1,173 @@ +/* + * Copyright 2023 Apple Inc. + * + * 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. + */ +package com.netflix.spinnaker.kork.artifacts.artifactstore.s3; + +import static com.netflix.spinnaker.kork.artifacts.artifactstore.s3.S3ArtifactStore.ENFORCE_PERMS_KEY; + +import com.netflix.spinnaker.kork.artifacts.ArtifactTypes; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactReferenceURI; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreStorer; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreURIBuilder; +import com.netflix.spinnaker.kork.artifacts.model.Artifact; +import com.netflix.spinnaker.kork.exceptions.SpinnakerException; +import com.netflix.spinnaker.security.AuthenticatedRequest; +import java.util.Base64; +import java.util.regex.Pattern; +import lombok.extern.log4j.Log4j2; +import org.apache.http.HttpStatus; +import software.amazon.awssdk.core.sync.RequestBody; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.HeadObjectRequest; +import software.amazon.awssdk.services.s3.model.NoSuchKeyException; +import software.amazon.awssdk.services.s3.model.PutObjectRequest; +import software.amazon.awssdk.services.s3.model.S3Exception; +import software.amazon.awssdk.services.s3.model.Tag; +import software.amazon.awssdk.services.s3.model.Tagging; + +/** + * S3ArtifactStoreStorer will store artifacts in a s3 compatible service + * + *

Note: It is very important that the S3 bucket has object lock on it to prevent multiple writes + * {@see https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-lock-overview.html} + */ +@Log4j2 +public class S3ArtifactStoreStorer implements ArtifactStoreStorer { + private final S3Client s3Client; + private final String bucket; + private final ArtifactStoreURIBuilder uriBuilder; + private final String applicationsRegex; + + public S3ArtifactStoreStorer( + S3Client s3Client, + String bucket, + ArtifactStoreURIBuilder uriBuilder, + String applicationsRegex) { + this.s3Client = s3Client; + this.bucket = bucket; + this.uriBuilder = uriBuilder; + this.applicationsRegex = applicationsRegex; + } + + /** + * Will store the artifact using the {@link #s3Client} in some {@link #bucket} + * + *

This method also persists "permissions" by storing the execution id that made the original + * store call. In the event a service wants to retrieve said artifact, they will also need to + * provide the proper execution id + */ + @Override + public Artifact store(Artifact artifact) { + String application = AuthenticatedRequest.getSpinnakerApplication().orElse(null); + if (application == null) { + log.warn("failed to retrieve application from request artifact={}", artifact.getName()); + return artifact; + } + + if (applicationsRegex != null && !Pattern.matches(applicationsRegex, application)) { + return artifact; + } + + ArtifactReferenceURI ref = uriBuilder.buildArtifactURI(application, artifact); + Artifact remoteArtifact = + artifact.toBuilder() + .type(ArtifactTypes.REMOTE_BASE64.getMimeType()) + .reference(ref.uri()) + .build(); + + if (objectExists(ref)) { + return remoteArtifact; + } + + // purpose of tagging is to ensure some sort of identity is persisted to + // enforce permissions when retrieving the artifact + Tag accountTag = Tag.builder().key(ENFORCE_PERMS_KEY).value(application).build(); + + PutObjectRequest request = + PutObjectRequest.builder() + .bucket(bucket) + .key(ref.paths()) + .tagging(Tagging.builder().tagSet(accountTag).build()) + .build(); + + s3Client.putObject(request, RequestBody.fromBytes(getReferenceAsBytes(artifact))); + return remoteArtifact; + } + + private byte[] getReferenceAsBytes(Artifact artifact) { + String reference = artifact.getReference(); + if (reference == null) { + throw new IllegalArgumentException("reference cannot be null"); + } + + String type = artifact.getType(); + if (type != null && type.endsWith("/base64")) { + return Base64.getDecoder().decode(reference); + } + + return reference.getBytes(); + } + + /** + * Helper method to check whether the object exists. This is not thread safe, nor would it help in + * a distributed system due to how S3 works (no conditional statements). If preventing multiple + * writes of the same object is important, another filestore/db needs to be used, possibly + * dynamodb. + */ + private boolean objectExists(ArtifactReferenceURI uri) { + HeadObjectRequest request = HeadObjectRequest.builder().bucket(bucket).key(uri.paths()).build(); + try { + s3Client.headObject(request); + log.debug("Artifact exists. No need to store. reference={}", uri.uri()); + return true; + } catch (NoSuchKeyException e) { + // pretty gross that we need to use exceptions as control flow, but the + // java SDK doesn't have any other way of check if an object exists in s3 + log.info("Artifact does not exist reference={}", uri.uri()); + return false; + } catch (S3Exception e) { + int statusCode = e.statusCode(); + log.error( + "Artifact store failed head object request statusCode={} reference={}", + statusCode, + uri.uri()); + + if (statusCode != 0) { + // due to this being a HEAD request, there is no message giving a clear + // indication of what failed. Rather than seeing a useful message back + // to gate, we instead see just null. To alleviate this, we wrap the + // exception with a more meaningful message + throw new SpinnakerException(buildHeadObjectExceptionMessage(e), e); + } + + throw new SpinnakerException("S3 head object failed", e); + } + } + + /** + * S3's head object can only return 400, 403, and 404, and based on the HTTP status code, we will + * return the appropriate message back + */ + private static String buildHeadObjectExceptionMessage(S3Exception e) { + switch (e.statusCode()) { + case HttpStatus.SC_FORBIDDEN: + return "Failed to query artifact due to IAM permissions either on the bucket or object"; + case HttpStatus.SC_BAD_REQUEST: + return "Failed to query artifact due to invalid request"; + default: + return String.format("Failed to query artifact: %d", e.statusCode()); + } + } +} diff --git a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/model/ExpectedArtifact.java b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/model/ExpectedArtifact.java index 3debe5bfc..cabbc8a9c 100644 --- a/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/model/ExpectedArtifact.java +++ b/kork-artifacts/src/main/java/com/netflix/spinnaker/kork/artifacts/model/ExpectedArtifact.java @@ -98,7 +98,23 @@ public boolean matches(Artifact other) { } private boolean matches(@Nullable String us, @Nullable String other) { - return StringUtils.isEmpty(us) || (other != null && patternMatches(us, other)); + if (StringUtils.isEmpty(us)) { + return true; + } + + if (other == null) { + return false; + } + + // The strict equals is mostly to ensure base64 references can be compared + // against each other, since the '+' is a completely valid base64 character. + // The '+' in regex has the meaning of one or more, and will change the + // semantics of comparing two references. + // + // So rather than having references implement its own matching, users may + // rely on matching artifacts with some regex. To be backwards compatible we + // will do a strict comparison as well as pattern matching. + return us.equals(other) || patternMatches(us, other); } /** diff --git a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactDeserializerTest.java b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactDeserializerTest.java index 89758db60..25b369f3d 100644 --- a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactDeserializerTest.java +++ b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactDeserializerTest.java @@ -28,23 +28,43 @@ import org.junit.jupiter.api.Test; class ArtifactDeserializerTest { - private class InMemoryArtifactStore extends ArtifactStore { - public Map storageMap = new HashMap<>(); + private class InMemoryArtifactStore { + private final Map storageMap = new HashMap<>(); public InMemoryArtifactStore put(String id, Artifact artifact) { storageMap.put(id, artifact); return this; } + public Artifact get(String id) { + return storageMap.get(id); + } + } + + private class InMemoryArtifactStoreStorer implements ArtifactStoreStorer { + public final InMemoryArtifactStore inMemoryArtifactStore; + + public InMemoryArtifactStoreStorer(InMemoryArtifactStore inMemoryArtifactStore) { + this.inMemoryArtifactStore = inMemoryArtifactStore; + } + @Override public Artifact store(Artifact artifact) { - storageMap.put(artifact.getReference(), artifact); + inMemoryArtifactStore.put(artifact.getReference(), artifact); return artifact; } + } + + private class InMemoryArtifactStoreGetter implements ArtifactStoreGetter { + public final InMemoryArtifactStore inMemoryArtifactStore; + + public InMemoryArtifactStoreGetter(InMemoryArtifactStore inMemoryArtifactStore) { + this.inMemoryArtifactStore = inMemoryArtifactStore; + } @Override public Artifact get(ArtifactReferenceURI uri, ArtifactDecorator... decorator) { - return storageMap.get(uri.uri()); + return inMemoryArtifactStore.get(uri.uri()); } } @@ -58,7 +78,12 @@ public void simpleDeserialization() throws IOException { .type(ArtifactTypes.REMOTE_BASE64.getMimeType()) .reference(expectedReference) .build(); - InMemoryArtifactStore storage = new InMemoryArtifactStore().put("ref://link", expectedArtifact); + InMemoryArtifactStore inMemoryArtifactStore = + new InMemoryArtifactStore().put("ref://link", expectedArtifact); + ArtifactStore storage = + new ArtifactStore( + new InMemoryArtifactStoreGetter(inMemoryArtifactStore), + new InMemoryArtifactStoreStorer(inMemoryArtifactStore)); ArtifactDeserializer deserializer = new ArtifactDeserializer(new ObjectMapper(), storage); // We avoid using an object mapper here since the Artifact class has a diff --git a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfigurationTest.java b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfigurationTest.java new file mode 100644 index 000000000..448fef717 --- /dev/null +++ b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreConfigurationTest.java @@ -0,0 +1,48 @@ +/* + * Copyright 2023 Salesforce, Inc. + * + * 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. + */ + +package com.netflix.spinnaker.kork.artifacts.artifactstore; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.springframework.boot.context.annotation.UserConfigurations; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; + +class ArtifactStoreConfigurationTest { + + private final ApplicationContextRunner runner = + new ApplicationContextRunner() + .withConfiguration(UserConfigurations.of(ArtifactStoreConfiguration.class)); + + @BeforeEach + void init(TestInfo testInfo) { + System.out.println("--------------- Test " + testInfo.getDisplayName()); + } + + @Test + void testArtifactStoreDefaults() { + runner.run( + ctx -> { + assertThat(ctx).hasSingleBean(ArtifactStoreURIBuilder.class); + assertThat(ctx).hasSingleBean(ArtifactStore.class); + assertThat(ctx).hasSingleBean(NoopArtifactStoreGetter.class); + assertThat(ctx).hasSingleBean(NoopArtifactStoreStorer.class); + }); + } +} diff --git a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreTest.java b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreTest.java new file mode 100644 index 000000000..51fe2c572 --- /dev/null +++ b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/ArtifactStoreTest.java @@ -0,0 +1,60 @@ +/* + * Copyright 2023 Salesforce, Inc. + * + * 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. + */ + +package com.netflix.spinnaker.kork.artifacts.artifactstore; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import com.netflix.spinnaker.kork.artifacts.model.Artifact; +import org.junit.jupiter.api.Test; + +class ArtifactStoreTest { + + private ArtifactStoreGetter artifactStoreGetter = mock(ArtifactStoreGetter.class); + private ArtifactStoreStorer artifactStoreStorer = mock(ArtifactStoreStorer.class); + private ArtifactStore artifactStore = new ArtifactStore(artifactStoreGetter, artifactStoreStorer); + + @Test + void testArtifactStoreDelegatesToGetter() { + ArtifactReferenceURI uri = mock(ArtifactReferenceURI.class); + ArtifactDecorator artifactDecorator = mock(ArtifactDecorator.class); + + artifactStore.get(uri, artifactDecorator); + + verify(artifactStoreGetter).get(uri, artifactDecorator); + verifyNoMoreInteractions(artifactStoreGetter); + verifyNoInteractions(artifactStoreStorer); + } + + @Test + void testArtifactStoreDelegatesToStorer() { + Artifact inputArtifact = Artifact.builder().build(); + Artifact storedArtifact = Artifact.builder().build(); + when(artifactStore.store(inputArtifact)).thenReturn(storedArtifact); + + Artifact retval = artifactStore.store(inputArtifact); + assertThat(retval).isEqualTo(storedArtifact); + + verify(artifactStoreStorer).store(inputArtifact); + verifyNoMoreInteractions(artifactStoreStorer); + verifyNoInteractions(artifactStoreGetter); + } +} diff --git a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/EmbeddedArtifactSerializerTest.java b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/EmbeddedArtifactSerializerTest.java index 007e01324..b76c2417a 100644 --- a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/EmbeddedArtifactSerializerTest.java +++ b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/EmbeddedArtifactSerializerTest.java @@ -23,7 +23,8 @@ import com.fasterxml.jackson.databind.module.SimpleModule; import com.netflix.spinnaker.kork.artifacts.ArtifactTypes; import com.netflix.spinnaker.kork.artifacts.artifactstore.exceptions.ArtifactStoreIOException; -import com.netflix.spinnaker.kork.artifacts.artifactstore.s3.S3ArtifactStore; +import com.netflix.spinnaker.kork.artifacts.artifactstore.s3.S3ArtifactStoreGetter; +import com.netflix.spinnaker.kork.artifacts.artifactstore.s3.S3ArtifactStoreStorer; import com.netflix.spinnaker.kork.artifacts.model.Artifact; import com.netflix.spinnaker.kork.common.Header; import com.netflix.spinnaker.security.AuthenticatedRequest; @@ -46,7 +47,7 @@ class EmbeddedArtifactSerializerTest { public void serializeEmbeddedBase64Artifact_test( String name, String expectedJson, Artifact artifact, Artifact mockArtifact) throws IOException { - ArtifactStore storage = Mockito.mock(S3ArtifactStore.class); + ArtifactStore storage = Mockito.mock(ArtifactStore.class); when(storage.store(Mockito.any())).thenReturn(mockArtifact); EmbeddedArtifactSerializer serializer = @@ -66,8 +67,11 @@ public void ensureS3ExceptionHasProperMessages() { when(client.headObject((HeadObjectRequest) Mockito.any())) .thenThrow(S3Exception.builder().statusCode(400).build()); AuthenticatedRequest.set(Header.APPLICATION, "my-application"); - S3ArtifactStore artifactStore = - new S3ArtifactStore(client, null, "my-bucket", new ArtifactStoreURISHA256Builder(), null); + ArtifactStoreGetter s3ArtifactStoreGetter = + new S3ArtifactStoreGetter(client, null, "my-bucket"); + ArtifactStoreStorer artifactStoreStorer = + new S3ArtifactStoreStorer(client, "my-bucket", new ArtifactStoreURISHA256Builder(), null); + ArtifactStore artifactStore = new ArtifactStore(s3ArtifactStoreGetter, artifactStoreStorer); EmbeddedArtifactSerializer serializer = new EmbeddedArtifactSerializer(new ObjectMapper(), artifactStore); diff --git a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreConfigurationTest.java b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreConfigurationTest.java new file mode 100644 index 000000000..9374573a1 --- /dev/null +++ b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreConfigurationTest.java @@ -0,0 +1,93 @@ +/* + * Copyright 2023 Salesforce, Inc. + * + * 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. + */ + +package com.netflix.spinnaker.kork.artifacts.artifactstore.s3; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStore; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreConfiguration; +import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreURIBuilder; +import com.netflix.spinnaker.kork.artifacts.artifactstore.NoopArtifactStoreGetter; +import com.netflix.spinnaker.kork.artifacts.artifactstore.NoopArtifactStoreStorer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.springframework.boot.context.annotation.UserConfigurations; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.test.util.ReflectionTestUtils; +import software.amazon.awssdk.services.s3.S3Client; + +class S3ArtifactStoreConfigurationTest { + + private final ApplicationContextRunner runner = + new ApplicationContextRunner() + .withConfiguration(UserConfigurations.of(ArtifactStoreConfiguration.class)); + + @BeforeEach + void init(TestInfo testInfo) { + System.out.println("--------------- Test " + testInfo.getDisplayName()); + } + + @AfterEach + void cleanup() { + // Clear ArtifactStore.instance so we don't leave lingering state for other + // tests that assume it's null. + // + // Note that ArtifactStore.setInstance(null) doesn't set instance to null + // if it's already set. + ReflectionTestUtils.setField(ArtifactStore.class, "instance", null); + } + + @Test + void testArtifactStoreS3Disabled() { + runner + .withPropertyValues( + "artifact-store.type=s3", + "artifact-store.s3.enabled=false", + "artifact-store.s3.region=us-west-2") // arbitrary region + .run( + ctx -> { + assertThat(ctx).hasSingleBean(ArtifactStoreURIBuilder.class); + assertThat(ctx).hasSingleBean(ArtifactStore.class); + assertThat(ctx).hasSingleBean(S3ArtifactStoreGetter.class); + assertThat(ctx).hasSingleBean(NoopArtifactStoreStorer.class); + assertThat(ctx).doesNotHaveBean(NoopArtifactStoreGetter.class); + assertThat(ctx).doesNotHaveBean(S3ArtifactStoreStorer.class); + assertThat(ctx).hasSingleBean(S3Client.class); + }); + } + + @Test + void testArtifactStoreS3Enabled() { + runner + .withPropertyValues( + "artifact-store.type=s3", + "artifact-store.s3.enabled=true", + "artifact-store.s3.region=us-west-2") // arbitrary region + .run( + ctx -> { + assertThat(ctx).hasSingleBean(ArtifactStoreURIBuilder.class); + assertThat(ctx).hasSingleBean(ArtifactStore.class); + assertThat(ctx).hasSingleBean(S3ArtifactStoreGetter.class); + assertThat(ctx).hasSingleBean(S3ArtifactStoreStorer.class); + assertThat(ctx).doesNotHaveBean(NoopArtifactStoreGetter.class); + assertThat(ctx).doesNotHaveBean(NoopArtifactStoreStorer.class); + assertThat(ctx).hasSingleBean(S3Client.class); + }); + } +} diff --git a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreTest.java b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreStorerTest.java similarity index 92% rename from kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreTest.java rename to kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreStorerTest.java index 8564517fa..e1d8f9790 100644 --- a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreTest.java +++ b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/artifactstore/s3/S3ArtifactStoreStorerTest.java @@ -33,21 +33,21 @@ import software.amazon.awssdk.services.s3.model.HeadObjectRequest; import software.amazon.awssdk.services.s3.model.S3Exception; -public class S3ArtifactStoreTest { +public class S3ArtifactStoreStorerTest { @Test public void testExceptionPathOfObjectExists() { S3Client client = mock(S3Client.class); when(client.headObject((HeadObjectRequest) Mockito.any())) .thenThrow(S3Exception.builder().statusCode(400).build()); AuthenticatedRequest.set(Header.APPLICATION, "my-application"); - S3ArtifactStore artifactStore = - new S3ArtifactStore(client, null, "my-bucket", new ArtifactStoreURISHA256Builder(), null); + S3ArtifactStoreStorer artifactStoreStorer = + new S3ArtifactStoreStorer(client, "my-bucket", new ArtifactStoreURISHA256Builder(), null); String expectedExceptionMessage = "Failed to query artifact due to invalid request"; SpinnakerException e = Assertions.assertThrows( SpinnakerException.class, () -> { - artifactStore.store( + artifactStoreStorer.store( Artifact.builder() .type(ArtifactTypes.EMBEDDED_BASE64.getMimeType()) .reference("aGVsbG8gd29ybGQK") diff --git a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/model/ExpectedArtifactTest.java b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/model/ExpectedArtifactTest.java index 596e6b906..48899833f 100644 --- a/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/model/ExpectedArtifactTest.java +++ b/kork-artifacts/src/test/java/com/netflix/spinnaker/kork/artifacts/model/ExpectedArtifactTest.java @@ -189,4 +189,37 @@ private static Stream noMatchConstructors() { s -> Artifact.builder().artifactAccount(s).build()) .map(Arguments::of); } + + @ParameterizedTest + @MethodSource("referenceCases") + void testReferenceMatches( + String matchReference, String otherReference, boolean shouldMatch, String caseName) { + ExpectedArtifact expectedArtifact = + ExpectedArtifact.builder() + .id("test") + .matchArtifact( + Artifact.builder() + .type(ArtifactTypes.EMBEDDED_BASE64.getMimeType()) + .reference(matchReference) + .build()) + .build(); + + assertThat( + expectedArtifact.matches( + Artifact.builder() + .type(ArtifactTypes.EMBEDDED_BASE64.getMimeType()) + .reference(otherReference) + .build())) + .as(caseName) + .isEqualTo(shouldMatch); + } + + private static Stream referenceCases() { + return Stream.of( + Arguments.of("SGVsbG8gV29ybGQK", "SGVsbG8gV29ybGQK", true, "simple"), + Arguments.of("SGVsbG8gV29ybGQK", null, false, "other reference as null"), + Arguments.of("SGVsbG8gV29ybGQK", "", false, "other reference is empty"), + Arguments.of("", "SGVsbG8gV29ybGQK", true, "match reference is empty"), + Arguments.of("+++", "+++", true, "valid base64 but invalid regex")); + } } diff --git a/kork-core/kork-core.gradle b/kork-core/kork-core.gradle index 064452767..c3146d1e6 100644 --- a/kork-core/kork-core.gradle +++ b/kork-core/kork-core.gradle @@ -30,6 +30,5 @@ dependencies { testImplementation "org.spockframework:spock-core" testRuntimeOnly "cglib:cglib-nodep" testRuntimeOnly "org.objenesis:objenesis" - testRuntimeOnly "org.slf4j:slf4j-simple" testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine" } diff --git a/kork-core/src/main/java/com/netflix/spinnaker/kork/common/Header.java b/kork-core/src/main/java/com/netflix/spinnaker/kork/common/Header.java index 257d0b333..2ee731034 100644 --- a/kork-core/src/main/java/com/netflix/spinnaker/kork/common/Header.java +++ b/kork-core/src/main/java/com/netflix/spinnaker/kork/common/Header.java @@ -31,6 +31,8 @@ public enum Header { EXECUTION_ID("X-SPINNAKER-EXECUTION-ID", false), EXECUTION_TYPE("X-SPINNAKER-EXECUTION-TYPE", false), APPLICATION("X-SPINNAKER-APPLICATION", false), + ACCOUNT("X-SPINNAKER-ACCOUNT", false), + CLOUD_PROVIDER("X-SPINNAKER-CLOUD-PROVIDER", false), PLUGIN_ID("X-SPINNAKER-PLUGIN-ID", false), PLUGIN_EXTENSION("X-SPINNAKER-PLUGIN-EXTENSION", false); diff --git a/kork-credentials-api/kork-credentials-api.gradle b/kork-credentials-api/kork-credentials-api.gradle index 74f70a22c..83478be42 100644 --- a/kork-credentials-api/kork-credentials-api.gradle +++ b/kork-credentials-api/kork-credentials-api.gradle @@ -19,6 +19,7 @@ apply from: "$rootDir/gradle/lombok.gradle" dependencies { implementation(platform(project(":spinnaker-dependencies"))) + api project(":kork-plugins-api") implementation project(":kork-annotations") implementation project(":kork-exceptions") implementation 'javax.annotation:javax.annotation-api' diff --git a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/CredentialsLifecycleHandler.java b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/CredentialsLifecycleHandler.java index 374c9defe..4b8045c0b 100644 --- a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/CredentialsLifecycleHandler.java +++ b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/CredentialsLifecycleHandler.java @@ -16,6 +16,8 @@ package com.netflix.spinnaker.credentials; +import com.netflix.spinnaker.kork.plugins.api.internal.SpinnakerExtensionPoint; + /** * After {@link Credentials} have been parsed, they can be activated, refreshed, or retired - e.g. * adding agents. This happens before credentials are added or updated in the {@link @@ -23,7 +25,8 @@ * * @param */ -public interface CredentialsLifecycleHandler { +public interface CredentialsLifecycleHandler + extends SpinnakerExtensionPoint { /** * Credentials have been added. This is called before credentials are available in {@link diff --git a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/CredentialsRepository.java b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/CredentialsRepository.java index a7f370d19..b085f74fc 100644 --- a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/CredentialsRepository.java +++ b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/CredentialsRepository.java @@ -16,6 +16,7 @@ package com.netflix.spinnaker.credentials; +import com.netflix.spinnaker.kork.plugins.api.internal.SpinnakerExtensionPoint; import java.util.Set; import javax.annotation.Nullable; @@ -24,7 +25,7 @@ * * @param */ -public interface CredentialsRepository { +public interface CredentialsRepository extends SpinnakerExtensionPoint { /** * @param name * @return Credentials with the given name or null diff --git a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/AbstractCredentialsLoader.java b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/AbstractCredentialsLoader.java index 84749f06c..7d3d87b23 100644 --- a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/AbstractCredentialsLoader.java +++ b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/AbstractCredentialsLoader.java @@ -21,7 +21,8 @@ import javax.annotation.PostConstruct; import lombok.Getter; -public abstract class AbstractCredentialsLoader { +public abstract class AbstractCredentialsLoader + implements CredentialsLoader { @Getter protected final CredentialsRepository credentialsRepository; public AbstractCredentialsLoader(CredentialsRepository credentialsRepository) { @@ -34,5 +35,6 @@ public AbstractCredentialsLoader(CredentialsRepository credentialsRepository) * thereafter. */ @PostConstruct + @Override public abstract void load(); } diff --git a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsDefinitionSource.java b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsDefinitionSource.java index f09eb677d..20b88832f 100644 --- a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsDefinitionSource.java +++ b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsDefinitionSource.java @@ -16,6 +16,7 @@ package com.netflix.spinnaker.credentials.definition; +import com.netflix.spinnaker.kork.plugins.api.internal.SpinnakerExtensionPoint; import java.util.List; /** @@ -24,6 +25,7 @@ * * @param */ -public interface CredentialsDefinitionSource { +public interface CredentialsDefinitionSource + extends SpinnakerExtensionPoint { List getCredentialsDefinitions(); } diff --git a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsLoader.java b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsLoader.java new file mode 100644 index 000000000..8b40c1943 --- /dev/null +++ b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsLoader.java @@ -0,0 +1,27 @@ +/* + * Copyright 2023 Apple Inc. + * + * 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. + */ + +package com.netflix.spinnaker.credentials.definition; + +import com.netflix.spinnaker.credentials.Credentials; +import com.netflix.spinnaker.credentials.CredentialsRepository; +import com.netflix.spinnaker.kork.plugins.api.internal.SpinnakerExtensionPoint; + +public interface CredentialsLoader extends SpinnakerExtensionPoint { + CredentialsRepository getCredentialsRepository(); + + void load(); +} diff --git a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsParser.java b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsParser.java index 489a280d7..dad1b110c 100644 --- a/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsParser.java +++ b/kork-credentials-api/src/main/java/com/netflix/spinnaker/credentials/definition/CredentialsParser.java @@ -17,6 +17,7 @@ package com.netflix.spinnaker.credentials.definition; import com.netflix.spinnaker.credentials.Credentials; +import com.netflix.spinnaker.kork.plugins.api.internal.SpinnakerExtensionPoint; import javax.annotation.Nullable; /** @@ -25,7 +26,8 @@ * @param * @param */ -public interface CredentialsParser { +public interface CredentialsParser + extends SpinnakerExtensionPoint { /** Parses a definition into credentials. Can return null if the definition is to be ignored. */ @Nullable U parse(T credentials); diff --git a/kork-credentials/src/main/java/com/netflix/spinnaker/credentials/poller/Poller.java b/kork-credentials/src/main/java/com/netflix/spinnaker/credentials/poller/Poller.java index f19a61ce7..a814cca0a 100644 --- a/kork-credentials/src/main/java/com/netflix/spinnaker/credentials/poller/Poller.java +++ b/kork-credentials/src/main/java/com/netflix/spinnaker/credentials/poller/Poller.java @@ -17,7 +17,7 @@ package com.netflix.spinnaker.credentials.poller; import com.netflix.spinnaker.credentials.Credentials; -import com.netflix.spinnaker.credentials.definition.AbstractCredentialsLoader; +import com.netflix.spinnaker.credentials.definition.CredentialsLoader; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -34,7 +34,7 @@ @Slf4j @RequiredArgsConstructor public class Poller implements Runnable { - private final AbstractCredentialsLoader credentialsLoader; + private final CredentialsLoader credentialsLoader; public void run() { try { diff --git a/kork-credentials/src/main/java/com/netflix/spinnaker/credentials/poller/PollerConfiguration.java b/kork-credentials/src/main/java/com/netflix/spinnaker/credentials/poller/PollerConfiguration.java index 572e78b9f..90c888ebc 100644 --- a/kork-credentials/src/main/java/com/netflix/spinnaker/credentials/poller/PollerConfiguration.java +++ b/kork-credentials/src/main/java/com/netflix/spinnaker/credentials/poller/PollerConfiguration.java @@ -16,11 +16,12 @@ package com.netflix.spinnaker.credentials.poller; -import com.netflix.spinnaker.credentials.definition.AbstractCredentialsLoader; -import java.util.List; +import com.netflix.spinnaker.credentials.Credentials; +import com.netflix.spinnaker.credentials.definition.CredentialsLoader; +import com.netflix.spinnaker.kork.annotations.NonnullByDefault; import java.util.concurrent.TimeUnit; import lombok.RequiredArgsConstructor; -import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.scheduling.annotation.SchedulingConfigurer; import org.springframework.scheduling.config.ScheduledTaskRegistrar; @@ -28,10 +29,10 @@ @EnableConfigurationProperties(PollerConfigurationProperties.class) @RequiredArgsConstructor -@Slf4j +@NonnullByDefault public class PollerConfiguration implements SchedulingConfigurer { private final PollerConfigurationProperties config; - private final List> pollers; + private final ObjectProvider> pollers; @Override public void configureTasks(ScheduledTaskRegistrar taskRegistrar) { diff --git a/kork-crypto/kork-crypto.gradle b/kork-crypto/kork-crypto.gradle index fd5a4d9dc..201487335 100644 --- a/kork-crypto/kork-crypto.gradle +++ b/kork-crypto/kork-crypto.gradle @@ -23,11 +23,11 @@ dependencies { api project(":kork-annotations") - implementation "org.bouncycastle:bcpkix-jdk15on" + implementation "org.bouncycastle:bcpkix-jdk18on" implementation "org.springframework:spring-aop" testImplementation "org.springframework.boot:spring-boot-starter-test" - testFixturesApi "org.bouncycastle:bcpkix-jdk15on" + testFixturesApi "org.bouncycastle:bcpkix-jdk18on" testFixturesApi "org.springframework:spring-core" } diff --git a/kork-crypto/src/main/java/com/netflix/spinnaker/kork/crypto/CipherSuites.java b/kork-crypto/src/main/java/com/netflix/spinnaker/kork/crypto/CipherSuites.java new file mode 100644 index 000000000..6a082cbc3 --- /dev/null +++ b/kork-crypto/src/main/java/com/netflix/spinnaker/kork/crypto/CipherSuites.java @@ -0,0 +1,67 @@ +/* + * Copyright 2023 Apple Inc. + * + * 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. + */ + +package com.netflix.spinnaker.kork.crypto; + +import java.util.ArrayList; +import java.util.List; + +/** + * Provides a common source for lists of TLS cipher suite baselines. + * + * @see Mozilla Server Side TLS + * recommendations + */ +public final class CipherSuites { + private static final List REQUIRED = + List.of("TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384", "TLS_CHACHA20_POLY1305_SHA256"); + private static final List BROWSER_COMPATIBILITY = + List.of( + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256", + "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256"); + private static final List RESTRICTED = + List.of( + "TLS_DHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_DHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256"); + + /** + * Returns the list of baseline ciphers that should be enabled for TLS. These include the required + * ciphers for TLSv1.3. + * + * @see Modern + * compatibility recommendations + */ + public static List getRequiredCiphers() { + return REQUIRED; + } + + public static List getRecommendedCiphers() { + var ciphers = new ArrayList<>(getRequiredCiphers()); + ciphers.addAll(BROWSER_COMPATIBILITY); + return ciphers; + } + + public static List getIntermediateCompatibilityCiphers() { + var ciphers = getRecommendedCiphers(); + ciphers.addAll(RESTRICTED); + return ciphers; + } +} diff --git a/kork-expressions/src/main/java/com/netflix/spinnaker/kork/expressions/ArtifactUriToReferenceConverter.java b/kork-expressions/src/main/java/com/netflix/spinnaker/kork/expressions/ArtifactUriToReferenceConverter.java index bfe21ecb7..cacfbc635 100644 --- a/kork-expressions/src/main/java/com/netflix/spinnaker/kork/expressions/ArtifactUriToReferenceConverter.java +++ b/kork-expressions/src/main/java/com/netflix/spinnaker/kork/expressions/ArtifactUriToReferenceConverter.java @@ -55,7 +55,7 @@ public Object convertValue( return defaultTypeConverter.convertValue(value, sourceType, targetType); } - if (artifactStore == null || !artifactStore.isArtifactURI((String) value)) { + if (artifactStore == null || !ArtifactReferenceURI.is((String) value)) { return defaultTypeConverter.convertValue(value, sourceType, targetType); } diff --git a/kork-expressions/src/test/java/com/netflix/spinnaker/kork/expressions/ExpressionsSupportTest.java b/kork-expressions/src/test/java/com/netflix/spinnaker/kork/expressions/ExpressionsSupportTest.java index e4b524588..19024ec83 100644 --- a/kork-expressions/src/test/java/com/netflix/spinnaker/kork/expressions/ExpressionsSupportTest.java +++ b/kork-expressions/src/test/java/com/netflix/spinnaker/kork/expressions/ExpressionsSupportTest.java @@ -155,6 +155,10 @@ public void delegatesTypeConversion() { public class MockArtifactStore extends ArtifactStore { public Map cache = new HashMap<>(); + public MockArtifactStore() { + super(null, null); + } + @Override public Artifact store(Artifact artifact) { return null; diff --git a/kork-jedis-test/kork-jedis-test.gradle b/kork-jedis-test/kork-jedis-test.gradle index f47572afd..68aa64cef 100644 --- a/kork-jedis-test/kork-jedis-test.gradle +++ b/kork-jedis-test/kork-jedis-test.gradle @@ -4,5 +4,5 @@ dependencies { api(platform(project(":spinnaker-dependencies"))) api "redis.clients:jedis" - api "io.spinnaker.embedded-redis:embedded-redis" + implementation "org.testcontainers:testcontainers" } diff --git a/kork-jedis-test/src/main/java/com/netflix/spinnaker/kork/jedis/EmbeddedRedis.java b/kork-jedis-test/src/main/java/com/netflix/spinnaker/kork/jedis/EmbeddedRedis.java index 2f6c4fd48..b98400382 100644 --- a/kork-jedis-test/src/main/java/com/netflix/spinnaker/kork/jedis/EmbeddedRedis.java +++ b/kork-jedis-test/src/main/java/com/netflix/spinnaker/kork/jedis/EmbeddedRedis.java @@ -1,32 +1,23 @@ package com.netflix.spinnaker.kork.jedis; -import java.io.IOException; -import java.net.ServerSocket; -import java.net.URI; -import java.net.URISyntaxException; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.utility.DockerImageName; import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPool; -import redis.clients.jedis.util.Pool; -import redis.embedded.RedisServer; public class EmbeddedRedis implements AutoCloseable { - private final URI connection; - private final RedisServer redisServer; + private static final int REDIS_PORT = 6379; - private Pool jedis; + private final GenericContainer redisContainer; - private EmbeddedRedis(int port) throws IOException, URISyntaxException { - this.connection = URI.create(String.format("redis://127.0.0.1:%d/0", port)); - this.redisServer = - RedisServer.builder() - .port(port) - .setting("bind 127.0.0.1") - .setting("appendonly no") - .setting("save \"\"") - .setting("databases 1") - .build(); - this.redisServer.start(); + private JedisPool jedis; + + private EmbeddedRedis() { + redisContainer = + new GenericContainer<>(DockerImageName.parse("library/redis:5-alpine")) + .withExposedPorts(REDIS_PORT); + redisContainer.start(); } @Override @@ -35,20 +26,20 @@ public void close() { } public void destroy() { - try { - this.redisServer.stop(); - } catch (Exception e) { - throw new RuntimeException(e); - } + redisContainer.stop(); + } + + public String getHost() { + return redisContainer.getHost(); } public int getPort() { - return redisServer.ports().get(0); + return redisContainer.getMappedPort(REDIS_PORT); } - public Pool getPool() { + public JedisPool getPool() { if (jedis == null) { - jedis = new JedisPool(connection); + jedis = new JedisPool(getHost(), getPort()); } return jedis; } @@ -58,13 +49,6 @@ public Jedis getJedis() { } public static EmbeddedRedis embed() { - try { - ServerSocket serverSocket = new ServerSocket(0); - int port = serverSocket.getLocalPort(); - serverSocket.close(); - return new EmbeddedRedis(port); - } catch (IOException | URISyntaxException e) { - throw new RuntimeException("Failed to create embedded Redis", e); - } + return new EmbeddedRedis(); } } diff --git a/kork-plugins-spring-api/kork-plugins-spring-api.gradle b/kork-plugins-spring-api/kork-plugins-spring-api.gradle index 30afa4da4..202d2464c 100644 --- a/kork-plugins-spring-api/kork-plugins-spring-api.gradle +++ b/kork-plugins-spring-api/kork-plugins-spring-api.gradle @@ -18,7 +18,7 @@ apply plugin: "java-library" apply from: "${project.rootDir}/gradle/kotlin-test.gradle" dependencies { - implementation(platform(project(":spinnaker-dependencies"))) + api(platform(project(":spinnaker-dependencies"))) api project(":kork-plugins-api") api "org.springframework.boot:spring-boot-starter-web" diff --git a/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/PluginSecretTest.kt b/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/PluginSecretTest.kt index 0d8270201..18690d709 100644 --- a/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/PluginSecretTest.kt +++ b/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/PluginSecretTest.kt @@ -113,7 +113,7 @@ class PluginSecretTest : JUnit5Minutests { } @TestConfiguration - private class TestSecretEngineConfiguration { + class TestSecretEngineConfiguration { @Bean fun testSecretEngine(): SecretEngine = object : SecretEngine { override fun clearCache() = Unit diff --git a/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ComplexInjectionScenarioTest.kt b/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ComplexInjectionScenarioTest.kt index 4cb35894a..62be19eb9 100644 --- a/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ComplexInjectionScenarioTest.kt +++ b/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ComplexInjectionScenarioTest.kt @@ -115,14 +115,14 @@ class ComplexInjectionScenarioTest : JUnit5Minutests { @TestConfiguration @ComponentScan("com.netflix.spinnaker.kork.plugins.v2.scenarios.fixtures") - private class ComplexInjectionTestConfiguration { + class ComplexInjectionTestConfiguration { @Bean fun injectsPluginExtensions(testExtensions: List): InjectsPluginExtensions { return InjectsPluginExtensions(testExtensions) } } - private class InjectsPluginExtensions(val testExtensions: List) { + class InjectsPluginExtensions(val testExtensions: List) { @PostConstruct fun tryOutExtensions() { // Verify that extensions can be used immediately. diff --git a/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ServiceDependenciesScenarioTest.kt b/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ServiceDependenciesScenarioTest.kt index b08a15d10..038c04ae6 100644 --- a/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ServiceDependenciesScenarioTest.kt +++ b/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ServiceDependenciesScenarioTest.kt @@ -78,7 +78,7 @@ class ServiceDependenciesScenarioTest : JUnit5Minutests { @Configuration @ComponentScan("com.netflix.spinnaker.kork.plugins.v2.scenarios.fixtures") - private class TestApplicationConfiguration + class TestApplicationConfiguration companion object { private val GENERATED = testPlugin { diff --git a/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ServiceInjectionScenarioTest.kt b/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ServiceInjectionScenarioTest.kt index 077a5211e..e33739366 100644 --- a/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ServiceInjectionScenarioTest.kt +++ b/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/v2/scenarios/ServiceInjectionScenarioTest.kt @@ -73,14 +73,14 @@ class ServiceInjectionScenarioTest : JUnit5Minutests { } @TestConfiguration - private class ServiceInjectionTestConfiguration { + class ServiceInjectionTestConfiguration { @Bean fun injectsPluginExtensions(testExtensions: List): InjectsPluginExtensions { return InjectsPluginExtensions(testExtensions) } } - private class InjectsPluginExtensions(val testExtensions: List) { + class InjectsPluginExtensions(val testExtensions: List) { @PostConstruct fun tryOutExtensions() { // Verify that extensions can be used immediately. diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/ErrorHandlingExecutorCallAdapterFactory.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/ErrorHandlingExecutorCallAdapterFactory.java index 81ebacb99..ec3aec12a 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/ErrorHandlingExecutorCallAdapterFactory.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/ErrorHandlingExecutorCallAdapterFactory.java @@ -152,11 +152,12 @@ public Response execute() { return syncResp; } } catch (JsonProcessingException jpe) { - throw new SpinnakerConversionException("Failed to process response body", jpe); + throw new SpinnakerConversionException( + "Failed to process response body", jpe, delegate.request()); } catch (IOException e) { - throw new SpinnakerNetworkException(e); + throw new SpinnakerNetworkException(e, delegate.request()); } catch (Exception e) { - throw new SpinnakerServerException(e); + throw new SpinnakerServerException(e, delegate.request()); } throw new SpinnakerHttpException(syncResp, retrofit); } @@ -250,11 +251,11 @@ public void onFailure(Call call, final Throwable t) { SpinnakerServerException exception; if (t instanceof IOException) { - exception = new SpinnakerNetworkException(t); + exception = new SpinnakerNetworkException(t, call.request()); } else if (t instanceof SpinnakerHttpException) { exception = (SpinnakerHttpException) t; } else { - exception = new SpinnakerServerException(t); + exception = new SpinnakerServerException(t, call.request()); } final SpinnakerServerException finalException = exception; callbackExecutor.execute( diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/Retrofit2SyncCall.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/Retrofit2SyncCall.java index 44d69bcc2..189ef1018 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/Retrofit2SyncCall.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/Retrofit2SyncCall.java @@ -33,7 +33,7 @@ public static T execute(Call call) { try { return call.execute().body(); } catch (IOException e) { - throw new SpinnakerNetworkException(e); + throw new SpinnakerNetworkException(e, call.request()); } } } diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerConversionException.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerConversionException.java index cfd2e3a6b..a8a5bebfa 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerConversionException.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerConversionException.java @@ -16,14 +16,36 @@ package com.netflix.spinnaker.kork.retrofit.exceptions; +import okhttp3.Request; +import retrofit.RetrofitError; + /** Wraps an exception converting a successful retrofit http response body to its indicated type */ public class SpinnakerConversionException extends SpinnakerServerException { - public SpinnakerConversionException(String message, Throwable cause) { + /** + * Construct a SpinnakerServerException from retrofit2 with a message and cause (e.g. an exception + * converting a response to the specified type). + */ + public SpinnakerConversionException(String message, Throwable cause, Request request) { + super(message, cause, request); + setRetryable(false); + } + + /** + * Construct a SpinnakerConversionException from another SpinnakerConversionException (e.g. via + * newInstance). + */ + public SpinnakerConversionException(String message, SpinnakerConversionException cause) { super(message, cause); setRetryable(false); } + /** Construct a SpinnakerConversionException corresponding to a RetrofitError. */ + public SpinnakerConversionException(RetrofitError e) { + super(e); + setRetryable(false); + } + @Override public SpinnakerConversionException newInstance(String message) { return new SpinnakerConversionException(message, this); diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java index a67d561b5..7a705797f 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java @@ -18,7 +18,6 @@ import com.google.common.base.Preconditions; import com.netflix.spinnaker.kork.annotations.NullableByDefault; -import java.io.IOException; import java.lang.annotation.Annotation; import java.util.HashMap; import java.util.Map; @@ -57,8 +56,6 @@ public class SpinnakerHttpException extends SpinnakerServerException { private final Map responseBody; - private final String url; - private final int responseCode; /** @@ -67,6 +64,7 @@ public class SpinnakerHttpException extends SpinnakerServerException { */ private final String reason; + /** Construct a SpinnakerHttpException corresponding to a RetrofitError. */ public SpinnakerHttpException(RetrofitError e) { super(e); @@ -102,7 +100,6 @@ public SpinnakerHttpException(RetrofitError e) { if (responseBody != null) { tmpMessage = (String) responseBody.get("message"); } - url = e.getUrl(); responseCode = response.getStatus(); reason = response.getReason(); rawMessage = tmpMessage != null ? tmpMessage : reason; @@ -114,6 +111,7 @@ public SpinnakerHttpException(RetrofitError e) { */ public SpinnakerHttpException( retrofit2.Response retrofit2Response, retrofit2.Retrofit retrofit) { + super(retrofit2Response.raw().request()); this.response = null; this.retrofit2Response = retrofit2Response; if ((retrofit2Response.code() == HttpStatus.NOT_FOUND.value()) @@ -121,7 +119,6 @@ public SpinnakerHttpException( setRetryable(false); } responseBody = this.getErrorBodyAs(retrofit); - url = retrofit2Response.raw().request().url().toString(); responseCode = retrofit2Response.code(); reason = retrofit2Response.message(); this.rawMessage = @@ -160,7 +157,6 @@ public SpinnakerHttpException(String message, SpinnakerHttpException cause) { this.retrofit2Response = cause.retrofit2Response; rawMessage = null; this.responseBody = cause.responseBody; - this.url = cause.url; this.responseCode = cause.responseCode; this.reason = cause.reason; } @@ -198,7 +194,8 @@ public String getMessage() { return super.getMessage(); } - return String.format("Status: %s, URL: %s, Message: %s", responseCode, url, getRawMessage()); + return String.format( + "Status: %s, URL: %s, Message: %s", responseCode, this.getUrl(), getRawMessage()); } @Override @@ -210,19 +207,15 @@ public Map getResponseBody() { return this.responseBody; } - public String getUrl() { - return this.url; - } - public String getReason() { return this.reason; } /** - * HTTP response body converted to specified {@code type}. {@code null} if there is no response. + * HTTP error response body converted to the specified {@code type}. * - * @throws RuntimeException wrapping the underlying IOException if unable to convert the body to - * the specified {@code type}. + * @return null if there's no response or unable to convert the body to the specified {@code + * type}. */ private Map getErrorBodyAs(Retrofit retrofit) { if (retrofit2Response == null) { @@ -233,8 +226,14 @@ private Map getErrorBodyAs(Retrofit retrofit) { retrofit.responseBodyConverter(Map.class, new Annotation[0]); try { return converter.convert(retrofit2Response.errorBody()); - } catch (IOException e) { - throw new RuntimeException(e); + } catch (Exception e) { + log.debug( + "unable to convert response to map ({} {}, {})", + retrofit2Response.raw().request().method(), + retrofit2Response.code(), + retrofit2Response.raw().request().url(), + e); + return null; } } } diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerNetworkException.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerNetworkException.java index a51843d82..febf466e0 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerNetworkException.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerNetworkException.java @@ -17,19 +17,34 @@ package com.netflix.spinnaker.kork.retrofit.exceptions; import com.netflix.spinnaker.kork.annotations.NonnullByDefault; +import okhttp3.Request; import retrofit.RetrofitError; -/** Wraps an exception of kind {@link RetrofitError.Kind} NETWORK. */ +/** Represents a network error while attempting to execute a retrofit http client request. */ @NonnullByDefault public final class SpinnakerNetworkException extends SpinnakerServerException { - public SpinnakerNetworkException(Throwable cause) { - super(cause); + + /** + * Construct a SpinnakerNetworkException from retrofit2 with a cause (e.g. an exception sending a + * request or processing a response). + */ + public SpinnakerNetworkException(Throwable cause, Request request) { + super(cause, request); } - public SpinnakerNetworkException(String message, Throwable cause) { + /** + * Construct a SpinnakerNetworkException from another SpinnakerNetworkException (e.g. via + * newInstance). + */ + public SpinnakerNetworkException(String message, SpinnakerNetworkException cause) { super(message, cause); } + /** Construct a SpinnakerNetworkException corresponding to a RetrofitError. */ + public SpinnakerNetworkException(RetrofitError e) { + super(e); + } + @Override public SpinnakerNetworkException newInstance(String message) { return new SpinnakerNetworkException(message, this); diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandler.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandler.java index 8dace58aa..ca03cb643 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandler.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandler.java @@ -59,11 +59,11 @@ public Throwable handleError(RetrofitError e) { } return retval; case NETWORK: - return new SpinnakerNetworkException(e.getMessage(), e.getCause()); + return new SpinnakerNetworkException(e); case CONVERSION: - return new SpinnakerConversionException(e.getMessage(), e.getCause()); + return new SpinnakerConversionException(e); default: - return new SpinnakerServerException(e.getMessage(), e.getCause()); + return new SpinnakerServerException(e); } } diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerException.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerException.java index 654c164aa..2f6e4e6b2 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerException.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerException.java @@ -18,20 +18,63 @@ import com.netflix.spinnaker.kork.annotations.NonnullByDefault; import com.netflix.spinnaker.kork.exceptions.SpinnakerException; +import lombok.Getter; +import okhttp3.Request; +import retrofit.RetrofitError; /** Represents an error while attempting to execute a retrofit http client request. */ @NonnullByDefault public class SpinnakerServerException extends SpinnakerException { - public SpinnakerServerException(String message, Throwable cause) { - super(message, cause); + @Getter private final String url; + @Getter private final String httpMethod; + + /** Construct a SpinnakerServerException corresponding to a RetrofitError. */ + public SpinnakerServerException(RetrofitError e) { + super(e.getMessage(), e.getCause()); + url = e.getUrl(); + httpMethod = null; } - public SpinnakerServerException(Throwable cause) { + /** + * Construct a SpinnakerServerException from retrofit2 with no cause (e.g. a non-200 http + * response). + */ + public SpinnakerServerException(Request request) { + super(); + url = request.url().toString(); + httpMethod = request.method(); + } + + /** + * Construct a SpinnakerServerException from retrofit2 with a cause (e.g. an exception sending a + * request or processing a response). + */ + public SpinnakerServerException(Throwable cause, Request request) { super(cause); + this.url = request.url().toString(); + this.httpMethod = request.method(); } - public SpinnakerServerException() {} + /** + * Construct a SpinnakerServerException from retrofit2 with a message and cause (e.g. an exception + * converting a response to the specified type). + */ + public SpinnakerServerException(String message, Throwable cause, Request request) { + super(message, cause); + this.url = request.url().toString(); + this.httpMethod = request.method(); + } + + /** + * Construct a SpinnakerServerException from another SpinnakerServerException (e.g. via + * newInstance). + */ + public SpinnakerServerException(String message, SpinnakerServerException cause) { + super(message, cause); + this.url = cause.getUrl(); + this.httpMethod = cause.getHttpMethod(); + } @Override public SpinnakerServerException newInstance(String message) { diff --git a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/Retrofit2SyncCallTest.java b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/Retrofit2SyncCallTest.java index c114ea97d..b622734d0 100644 --- a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/Retrofit2SyncCallTest.java +++ b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/Retrofit2SyncCallTest.java @@ -16,38 +16,45 @@ package com.netflix.spinnaker.kork.retrofit.exceptions; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowableOfType; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall; import java.io.IOException; +import okhttp3.HttpUrl; +import okhttp3.Request; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; import retrofit2.Call; import retrofit2.Response; -public class Retrofit2SyncCallTest { +class Retrofit2SyncCallTest { @Test - public void testExecuteSuccss() throws IOException { - Call mockcall = Mockito.mock(Call.class); - when(mockcall.execute()).thenReturn(Response.success("testing")); - String execute = Retrofit2SyncCall.execute(mockcall); - assertEquals("testing", execute); + void testExecuteSuccess() throws IOException { + Call mockCall = mock(Call.class); + String responseBody = "testing"; + when(mockCall.execute()).thenReturn(Response.success(responseBody)); + String execute = Retrofit2SyncCall.execute(mockCall); + assertThat(execute).isEqualTo(responseBody); } @Test - public void testExecuteThrowException() throws IOException { - Call mockcall = Mockito.mock(Call.class); + void testExecuteThrowException() throws IOException { + Call mockCall = mock(Call.class); IOException ioException = new IOException("exception test"); - when(mockcall.execute()).thenThrow(ioException); - SpinnakerNetworkException networkEx = - assertThrows( - SpinnakerNetworkException.class, - () -> { - Retrofit2SyncCall.execute(mockcall); - }); - assertEquals(ioException, networkEx.getCause()); + when(mockCall.execute()).thenThrow(ioException); + + HttpUrl url = HttpUrl.parse("http://arbitrary-url"); + Request mockRequest = mock(Request.class); + when(mockCall.request()).thenReturn(mockRequest); + when(mockRequest.url()).thenReturn(url); + + SpinnakerNetworkException thrown = + catchThrowableOfType( + () -> Retrofit2SyncCall.execute(mockCall), SpinnakerNetworkException.class); + assertThat(thrown).hasCause(ioException); + assertThat(thrown.getUrl()).isEqualTo(url.toString()); } } diff --git a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java index 9d2c4774b..80fea2ceb 100644 --- a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java +++ b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java @@ -19,9 +19,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowable; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.gson.Gson; import com.netflix.spinnaker.kork.exceptions.SpinnakerException; @@ -31,6 +28,7 @@ import okhttp3.MediaType; import okhttp3.ResponseBody; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import retrofit.RetrofitError; import retrofit.client.Response; @@ -39,11 +37,11 @@ import retrofit2.Retrofit; import retrofit2.converter.jackson.JacksonConverterFactory; -public class SpinnakerHttpExceptionTest { +class SpinnakerHttpExceptionTest { private static final String CUSTOM_MESSAGE = "custom message"; @Test - public void testSpinnakerHttpExceptionFromRetrofitError() { + void testSpinnakerHttpExceptionFromRetrofitError() { String url = "http://localhost"; int statusCode = 200; String message = "arbitrary message"; @@ -66,10 +64,11 @@ public void testSpinnakerHttpExceptionFromRetrofitError() { .isEqualTo("Status: " + statusCode + ", URL: " + url + ", Message: " + message); assertThat(spinnakerHttpException.getUrl()).isEqualTo(url); assertThat(spinnakerHttpException.getReason()).isEqualTo(reason); + assertThat(spinnakerHttpException.getHttpMethod()).isNull(); } @Test - public void testSpinnakerHttpExceptionFromRetrofitException() { + void testSpinnakerHttpExceptionFromRetrofitException() { final String validJsonResponseBodyString = "{\"name\":\"test\"}"; ResponseBody responseBody = ResponseBody.create( @@ -86,19 +85,20 @@ public void testSpinnakerHttpExceptionFromRetrofitException() { assertThat(retrofit2Service.baseUrl().toString()).isEqualTo(url); SpinnakerHttpException notFoundException = new SpinnakerHttpException(response, retrofit2Service); - assertNotNull(notFoundException.getResponseBody()); + assertThat(notFoundException.getResponseBody()).isNotNull(); assertThat(notFoundException.getUrl()).isEqualTo(url); assertThat(notFoundException.getReason()) .isEqualTo("Response.error()"); // set by Response.error Map errorResponseBody = notFoundException.getResponseBody(); - assertEquals(errorResponseBody.get("name"), "test"); - assertEquals(HttpStatus.NOT_FOUND.value(), notFoundException.getResponseCode()); - assertTrue( - notFoundException.getMessage().contains(String.valueOf(HttpStatus.NOT_FOUND.value()))); + assertThat(errorResponseBody.get("name")).isEqualTo("test"); + assertThat(notFoundException.getResponseCode()).isEqualTo(HttpStatus.NOT_FOUND.value()); + assertThat(notFoundException) + .hasMessageContaining(String.valueOf(HttpStatus.NOT_FOUND.value())); + assertThat(notFoundException.getHttpMethod()).isEqualTo(HttpMethod.GET.toString()); } @Test - public void testSpinnakerHttpException_NewInstance() { + void testSpinnakerHttpException_NewInstance() { String url = "http://localhost"; String reason = "reason"; Response response = new Response(url, 200, reason, List.of(), null); @@ -108,10 +108,11 @@ public void testSpinnakerHttpException_NewInstance() { } catch (SpinnakerException e) { SpinnakerException newException = e.newInstance(CUSTOM_MESSAGE); - assertTrue(newException instanceof SpinnakerHttpException); - assertEquals(CUSTOM_MESSAGE, newException.getMessage()); - assertEquals(e, newException.getCause()); - assertEquals(response.getStatus(), ((SpinnakerHttpException) newException).getResponseCode()); + assertThat(newException).isInstanceOf(SpinnakerHttpException.class); + assertThat(newException).hasMessage(CUSTOM_MESSAGE); + assertThat(newException).hasCause(e); + assertThat(((SpinnakerHttpException) newException).getResponseCode()) + .isEqualTo(response.getStatus()); SpinnakerHttpException spinnakerHttpException = (SpinnakerHttpException) newException; assertThat(spinnakerHttpException.getUrl()).isEqualTo(url); assertThat(spinnakerHttpException.getReason()).isEqualTo(reason); diff --git a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofit2ErrorHandleTest.java b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofit2ErrorHandleTest.java index 75756395b..f82efd3a0 100644 --- a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofit2ErrorHandleTest.java +++ b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofit2ErrorHandleTest.java @@ -16,12 +16,8 @@ package com.netflix.spinnaker.kork.retrofit.exceptions; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowableOfType; import com.fasterxml.jackson.databind.ObjectMapper; import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory; @@ -35,12 +31,13 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import retrofit2.Call; import retrofit2.Retrofit; import retrofit2.converter.jackson.JacksonConverterFactory; -public class SpinnakerRetrofit2ErrorHandleTest { +class SpinnakerRetrofit2ErrorHandleTest { private static Retrofit2Service retrofit2Service; @@ -48,8 +45,10 @@ public class SpinnakerRetrofit2ErrorHandleTest { private static String responseBodyString; + private static String baseUrl = mockWebServer.url("/").toString(); + @BeforeAll - public static void setupOnce() throws Exception { + static void setupOnce() throws Exception { Map responseBodyMap = new HashMap<>(); responseBodyMap.put("timestamp", "123123123123"); @@ -58,7 +57,7 @@ public static void setupOnce() throws Exception { retrofit2Service = new Retrofit.Builder() - .baseUrl(mockWebServer.url("/").toString()) + .baseUrl(baseUrl) .client( new OkHttpClient.Builder() .callTimeout(1, TimeUnit.SECONDS) @@ -71,61 +70,66 @@ public static void setupOnce() throws Exception { } @AfterAll - public static void shutdownOnce() throws Exception { + static void shutdownOnce() throws Exception { mockWebServer.shutdown(); } @Test - public void testRetrofitNotFoundIsNotRetryable() { + void testRetrofitNotFoundIsNotRetryable() { mockWebServer.enqueue( new MockResponse() .setResponseCode(HttpStatus.NOT_FOUND.value()) .setBody(responseBodyString)); SpinnakerHttpException notFoundException = - assertThrows(SpinnakerHttpException.class, () -> retrofit2Service.getRetrofit2().execute()); - assertNotNull(notFoundException.getRetryable()); - assertFalse(notFoundException.getRetryable()); + catchThrowableOfType( + () -> retrofit2Service.getRetrofit2().execute(), SpinnakerHttpException.class); + assertThat(notFoundException.getRetryable()).isNotNull(); + assertThat(notFoundException.getRetryable()).isFalse(); + assertThat(notFoundException.getUrl()).isEqualTo(mockWebServer.url("/retrofit2").toString()); } @Test - public void testRetrofitBadRequestIsNotRetryable() { + void testRetrofitBadRequestIsNotRetryable() { mockWebServer.enqueue( new MockResponse() .setResponseCode(HttpStatus.NOT_FOUND.value()) .setBody(responseBodyString)); SpinnakerHttpException spinnakerHttpException = - assertThrows(SpinnakerHttpException.class, () -> retrofit2Service.getRetrofit2().execute()); - assertNotNull(spinnakerHttpException.getRetryable()); - assertFalse(spinnakerHttpException.getRetryable()); + catchThrowableOfType( + () -> retrofit2Service.getRetrofit2().execute(), SpinnakerHttpException.class); + assertThat(spinnakerHttpException.getRetryable()).isNotNull(); + assertThat(spinnakerHttpException.getRetryable()).isFalse(); + assertThat(spinnakerHttpException.getUrl()) + .isEqualTo(mockWebServer.url("/retrofit2").toString()); } @Test - public void testRetrofitOtherClientErrorHasNullRetryable() { + void testRetrofitOtherClientErrorHasNullRetryable() { mockWebServer.enqueue( new MockResponse().setResponseCode(HttpStatus.GONE.value()).setBody(responseBodyString)); SpinnakerHttpException spinnakerHttpException = - assertThrows(SpinnakerHttpException.class, () -> retrofit2Service.getRetrofit2().execute()); - assertNull(spinnakerHttpException.getRetryable()); + catchThrowableOfType( + () -> retrofit2Service.getRetrofit2().execute(), SpinnakerHttpException.class); + assertThat(spinnakerHttpException.getRetryable()).isNull(); + assertThat(spinnakerHttpException.getUrl()) + .isEqualTo(mockWebServer.url("/retrofit2").toString()); } @Test - public void testRetrofitSimpleSpinnakerNetworkException() { + void testRetrofitSimpleSpinnakerNetworkException() { mockWebServer.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.NO_RESPONSE)); - - assertThrows(SpinnakerNetworkException.class, () -> retrofit2Service.getRetrofit2().execute()); - } - - @Test - public void testRetrofitSimpleSpinnakerServerException() { - mockWebServer.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START)); - assertThrows(SpinnakerServerException.class, () -> retrofit2Service.getRetrofit2().execute()); + SpinnakerNetworkException spinnakerNetworkException = + catchThrowableOfType( + () -> retrofit2Service.getRetrofit2().execute(), SpinnakerNetworkException.class); + assertThat(spinnakerNetworkException.getUrl()) + .isEqualTo(mockWebServer.url("/retrofit2").toString()); } @Test - public void testResponseHeadersInException() { + void testResponseHeadersInException() { // Check response headers are retrievable from a SpinnakerHttpException mockWebServer.enqueue( @@ -134,35 +138,50 @@ public void testResponseHeadersInException() { .setBody(responseBodyString) .setHeader("Test", "true")); SpinnakerHttpException spinnakerHttpException = - assertThrows(SpinnakerHttpException.class, () -> retrofit2Service.getRetrofit2().execute()); - assertTrue(spinnakerHttpException.getHeaders().containsKey("Test")); - assertTrue(spinnakerHttpException.getHeaders().get("Test").contains("true")); + catchThrowableOfType( + () -> retrofit2Service.getRetrofit2().execute(), SpinnakerHttpException.class); + assertThat(spinnakerHttpException.getHeaders().containsKey("Test")).isTrue(); + assertThat(spinnakerHttpException.getHeaders().get("Test").contains("true")).isTrue(); + assertThat(spinnakerHttpException.getUrl()) + .isEqualTo(mockWebServer.url("/retrofit2").toString()); } @Test - public void testNotParameterizedException() { + void testHttpMethodInException() { + // Check http request method is retrievable from a SpinnakerHttpException + mockWebServer.enqueue( + new MockResponse() + .setResponseCode(HttpStatus.BAD_REQUEST.value()) + .setBody(responseBodyString)); + SpinnakerHttpException spinnakerHttpException = + catchThrowableOfType( + () -> retrofit2Service.deleteRetrofit2().execute(), SpinnakerHttpException.class); + assertThat(spinnakerHttpException.getUrl()) + .isEqualTo(mockWebServer.url("/retrofit2").toString()); + assertThat(spinnakerHttpException.getHttpMethod()).isEqualTo(HttpMethod.DELETE.toString()); + } + @Test + void testNotParameterizedException() { IllegalArgumentException illegalArgumentException = - assertThrows( - IllegalArgumentException.class, - () -> retrofit2Service.testNotParameterized().execute()); - - assertEquals( - "Call return type must be parameterized as Call or Call", - illegalArgumentException.getCause().getMessage()); + catchThrowableOfType( + () -> retrofit2Service.testNotParameterized().execute(), + IllegalArgumentException.class); + assertThat(illegalArgumentException.getCause().getMessage()) + .isEqualTo("Call return type must be parameterized as Call or Call"); } @Test - public void testWrongReturnTypeException() { + void testWrongReturnTypeException() { IllegalArgumentException illegalArgumentException = - assertThrows( - IllegalArgumentException.class, () -> retrofit2Service.testWrongReturnType().execute()); + catchThrowableOfType( + () -> retrofit2Service.testWrongReturnType().execute(), IllegalArgumentException.class); - assertEquals( - "Unable to create call adapter for interface com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofit2ErrorHandleTest$DummyWithExecute\n" - + " for method Retrofit2Service.testWrongReturnType", - illegalArgumentException.getMessage()); + assertThat(illegalArgumentException) + .hasMessage( + "Unable to create call adapter for interface com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofit2ErrorHandleTest$DummyWithExecute\n" + + " for method Retrofit2Service.testWrongReturnType"); } interface Retrofit2Service { @@ -174,6 +193,9 @@ interface Retrofit2Service { @retrofit2.http.GET("/retrofit2/wrongReturnType") DummyWithExecute testWrongReturnType(); + + @retrofit2.http.DELETE("/retrofit2") + Call deleteRetrofit2(); } @Test @@ -187,10 +209,34 @@ void testSpinnakerConversionException() { .setBody(invalidJsonTypeResponseBody)); SpinnakerConversionException spinnakerConversionException = - assertThrows( - SpinnakerConversionException.class, () -> retrofit2Service.getRetrofit2().execute()); + catchThrowableOfType( + () -> retrofit2Service.getRetrofit2().execute(), SpinnakerConversionException.class); + assertThat(spinnakerConversionException.getRetryable()).isNotNull(); + assertThat(spinnakerConversionException.getRetryable()).isFalse(); + assertThat(spinnakerConversionException).hasMessage("Failed to process response body"); + assertThat(spinnakerConversionException.getUrl()) + .isEqualTo(mockWebServer.url("/retrofit2").toString()); + } - assertEquals("Failed to process response body", spinnakerConversionException.getMessage()); + @Test + void testNonJsonHttpErrorResponse() { + + String invalidJsonTypeResponseBody = "{'errorResponse': 'Failure'"; + int responseCode = HttpStatus.INTERNAL_SERVER_ERROR.value(); + String url = baseUrl + "retrofit2"; + String reason = "Server Error"; + + mockWebServer.enqueue( + new MockResponse().setResponseCode(responseCode).setBody(invalidJsonTypeResponseBody)); + SpinnakerHttpException spinnakerHttpException = + catchThrowableOfType( + () -> retrofit2Service.getRetrofit2().execute(), SpinnakerHttpException.class); + assertThat(spinnakerHttpException.getResponseBody()).isNull(); + assertThat(spinnakerHttpException.getResponseCode()).isEqualTo(responseCode); + assertThat(spinnakerHttpException) + .hasMessage("Status: " + responseCode + ", URL: " + url + ", Message: " + reason); + assertThat(spinnakerHttpException.getUrl()).isEqualTo(url); + assertThat(spinnakerHttpException.getReason()).isEqualTo(reason); } interface DummyWithExecute { diff --git a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandlerTest.java b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandlerTest.java index 405c010b4..748fa7bc1 100644 --- a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandlerTest.java +++ b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandlerTest.java @@ -16,12 +16,9 @@ package com.netflix.spinnaker.kork.retrofit.exceptions; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.catchThrowableOfType; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; @@ -30,6 +27,7 @@ import java.util.Map; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.SocketPolicy; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -42,14 +40,14 @@ import retrofit.converter.JacksonConverter; import retrofit.http.GET; -public class SpinnakerRetrofitErrorHandlerTest { +class SpinnakerRetrofitErrorHandlerTest { private static RetrofitService retrofitService; private static final MockWebServer mockWebServer = new MockWebServer(); @BeforeAll - public static void setupOnce() throws Exception { + static void setupOnce() throws Exception { mockWebServer.start(); retrofitService = @@ -61,17 +59,24 @@ public static void setupOnce() throws Exception { } @AfterAll - public static void shutdownOnce() throws Exception { + static void shutdownOnce() throws Exception { mockWebServer.shutdown(); } @Test - public void testNotFoundIsNotRetryable() { + void testNotFoundIsNotRetryable() { mockWebServer.enqueue(new MockResponse().setResponseCode(HttpStatus.NOT_FOUND.value())); SpinnakerHttpException notFoundException = - assertThrows(SpinnakerHttpException.class, () -> retrofitService.getFoo()); - assertNotNull(notFoundException.getRetryable()); - assertFalse(notFoundException.getRetryable()); + catchThrowableOfType(() -> retrofitService.getFoo(), SpinnakerHttpException.class); + assertThat(notFoundException.getRetryable()).isNotNull(); + assertThat(notFoundException.getRetryable()).isFalse(); + } + + @Test + void testSpinnakerNetworkException() { + mockWebServer.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.NO_RESPONSE)); + assertThatExceptionOfType(SpinnakerNetworkException.class) + .isThrownBy(() -> retrofitService.getFoo()); } @ParameterizedTest(name = "Deserialize response using {0}") @@ -88,7 +93,7 @@ public void testNotFoundIsNotRetryable() { // is set when building out the RestAdapter @ValueSource( strings = {"Default_GSONConverter", "JacksonConverter", "JacksonConverterWithObjectMapper"}) - public void testResponseWithExtraField(String retrofitConverter) throws Exception { + void testResponseWithExtraField(String retrofitConverter) throws Exception { Map responseBodyMap = new HashMap<>(); responseBodyMap.put("timestamp", "123123123123"); responseBodyMap.put("message", "Not Found error Message"); @@ -131,82 +136,108 @@ public void testResponseWithExtraField(String retrofitConverter) throws Exceptio // "..." // // so make sure we get a SpinnakerHttpException from calling getFoo - assertThrows(SpinnakerHttpException.class, retrofitServiceTestConverter::getFoo); + SpinnakerHttpException spinnakerHttpException = + catchThrowableOfType(retrofitServiceTestConverter::getFoo, SpinnakerHttpException.class); + assertThat(spinnakerHttpException.getUrl()).isEqualTo(mockWebServer.url("/foo").toString()); } @Test - public void testBadRequestIsNotRetryable() { + void testBadRequestIsNotRetryable() { mockWebServer.enqueue(new MockResponse().setResponseCode(HttpStatus.BAD_REQUEST.value())); SpinnakerHttpException spinnakerHttpException = - assertThrows(SpinnakerHttpException.class, () -> retrofitService.getFoo()); - assertNotNull(spinnakerHttpException.getRetryable()); - assertFalse(spinnakerHttpException.getRetryable()); + catchThrowableOfType(() -> retrofitService.getFoo(), SpinnakerHttpException.class); + assertThat(spinnakerHttpException.getRetryable()).isNotNull(); + assertThat(spinnakerHttpException.getRetryable()).isFalse(); + assertThat(spinnakerHttpException.getUrl()).isEqualTo(mockWebServer.url("/foo").toString()); } @Test - public void testOtherClientErrorHasNullRetryable() { + void testOtherClientErrorHasNullRetryable() { // Arbitrarily choose GONE as an example of a client (e.g. 4xx) error that // we expect to have null retryable mockWebServer.enqueue(new MockResponse().setResponseCode(HttpStatus.GONE.value())); SpinnakerHttpException spinnakerHttpException = - assertThrows(SpinnakerHttpException.class, () -> retrofitService.getFoo()); - assertNull(spinnakerHttpException.getRetryable()); + catchThrowableOfType(() -> retrofitService.getFoo(), SpinnakerHttpException.class); + assertThat(spinnakerHttpException.getRetryable()).isNull(); + assertThat(spinnakerHttpException.getUrl()).isEqualTo(mockWebServer.url("/foo").toString()); } @Test - public void testResponseHeadersInException() { + void testResponseHeadersInException() { // Check response headers are retrievable from a SpinnakerHttpException mockWebServer.enqueue( new MockResponse() .setResponseCode(HttpStatus.BAD_REQUEST.value()) .setHeader("Test", "true")); SpinnakerHttpException spinnakerHttpException = - assertThrows(SpinnakerHttpException.class, () -> retrofitService.getFoo()); - assertTrue(spinnakerHttpException.getHeaders().containsKey("Test")); - assertTrue(spinnakerHttpException.getHeaders().get("Test").contains("true")); + catchThrowableOfType(() -> retrofitService.getFoo(), SpinnakerHttpException.class); + assertThat(spinnakerHttpException.getHeaders().containsKey("Test")).isTrue(); + assertThat(spinnakerHttpException.getHeaders().get("Test").contains("true")).isTrue(); + assertThat(spinnakerHttpException.getUrl()).isEqualTo(mockWebServer.url("/foo").toString()); } @Test - public void testSimpleSpinnakerNetworkException() { + void testExceptionFromRetrofitErrorHasNullHttpMethod() { + mockWebServer.enqueue( + new MockResponse() + .setResponseCode(HttpStatus.BAD_REQUEST.value()) + .setHeader("Test", "true")); + SpinnakerHttpException spinnakerHttpException = + catchThrowableOfType(() -> retrofitService.getFoo(), SpinnakerHttpException.class); + assertThat(spinnakerHttpException.getHttpMethod()).isNull(); + } + + @Test + void testSimpleSpinnakerNetworkException() { String message = "my custom message"; IOException e = new IOException(message); - RetrofitError retrofitError = RetrofitError.networkError("http://localhost", e); + String url = "http://localhost"; + RetrofitError retrofitError = RetrofitError.networkError(url, e); SpinnakerRetrofitErrorHandler handler = SpinnakerRetrofitErrorHandler.getInstance(); Throwable throwable = handler.handleError(retrofitError); - assertEquals(message, throwable.getMessage()); + assertThat(throwable).hasMessage(message); + assertThat(throwable).isInstanceOf(SpinnakerNetworkException.class); + SpinnakerNetworkException spinnakerNetworkException = (SpinnakerNetworkException) throwable; + assertThat(spinnakerNetworkException.getUrl()).isEqualTo(url); } @Test - public void testSpinnakerConversionException() { + void testSpinnakerConversionException() { mockWebServer.enqueue( new MockResponse().setBody("Invalid JSON response").setResponseCode(HttpStatus.OK.value())); SpinnakerConversionException spinnakerConversionException = - assertThrows(SpinnakerConversionException.class, () -> retrofitService.getData()); - assertTrue( - spinnakerConversionException - .getMessage() - .contains("Expected BEGIN_OBJECT but was STRING at line 1 column 1 path $")); + catchThrowableOfType(() -> retrofitService.getData(), SpinnakerConversionException.class); + assertThat(spinnakerConversionException.getRetryable()).isNotNull(); + assertThat(spinnakerConversionException.getRetryable()).isFalse(); + assertThat(spinnakerConversionException) + .hasMessageContaining("Expected BEGIN_OBJECT but was STRING at line 1 column 1 path $"); + assertThat(spinnakerConversionException.getUrl()) + .isEqualTo(mockWebServer.url("/data").toString()); } @Test - public void testChainSpinnakerException_SpinnakerNetworkException() { + void testChainSpinnakerException_SpinnakerNetworkException() { SpinnakerRetrofitErrorHandler handler = SpinnakerRetrofitErrorHandler.getInstance(); String originalMessage = "original message"; String newMessage = "new message"; IOException originalException = new IOException(originalMessage); - RetrofitError retrofitError = RetrofitError.networkError("http://localhost", originalException); + + String url = "http://localhost"; + RetrofitError retrofitError = RetrofitError.networkError(url, originalException); Throwable newException = handler.handleError( retrofitError, (exception) -> String.format("%s: %s", newMessage, exception.getMessage())); - assertTrue(newException instanceof SpinnakerNetworkException); - assertEquals("new message: original message", newException.getMessage()); - assertEquals(originalMessage, newException.getCause().getMessage()); + assertThat(newException).isInstanceOf(SpinnakerNetworkException.class); + assertThat(newException).hasMessage("new message: original message"); + assertThat(newException.getCause()).hasMessage(originalMessage); + SpinnakerNetworkException spinnakerNetworkException = (SpinnakerNetworkException) newException; + assertThat(spinnakerNetworkException.getUrl()).isEqualTo(url); } interface RetrofitService { diff --git a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitExceptionHandlersTest.java b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitExceptionHandlersTest.java index cb342b20e..93bbc2ad5 100644 --- a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitExceptionHandlersTest.java +++ b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitExceptionHandlersTest.java @@ -16,7 +16,7 @@ package com.netflix.spinnaker.kork.retrofit.exceptions; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -74,7 +74,7 @@ class SpinnakerRetrofitExceptionHandlersTest { private MemoryAppender memoryAppender; @BeforeEach - private void setup(TestInfo testInfo) { + void setup(TestInfo testInfo) { System.out.println("--------------- Test " + testInfo.getDisplayName()); memoryAppender = new MemoryAppender(SpinnakerRetrofitExceptionHandlers.class); } @@ -84,8 +84,8 @@ void testSpinnakerServerException() throws Exception { URI uri = getUri("/spinnakerServerException"); ResponseEntity entity = restTemplate.getForEntity(uri, String.class); - assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, entity.getStatusCode()); - assertEquals(1, memoryAppender.countEventsForLevel(Level.ERROR)); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); + assertThat(memoryAppender.countEventsForLevel(Level.ERROR)).isEqualTo(1); } @Test @@ -93,11 +93,11 @@ void testChainedSpinnakerServerException() throws Exception { URI uri = getUri("/chainedSpinnakerServerException"); ResponseEntity entity = restTemplate.getForEntity(uri, String.class); - assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, entity.getStatusCode()); - assertEquals(1, memoryAppender.countEventsForLevel(Level.ERROR)); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); + assertThat(memoryAppender.countEventsForLevel(Level.ERROR)).isEqualTo(1); // Make sure the message is what we expect. - assertEquals(1, memoryAppender.search(CUSTOM_MESSAGE, Level.ERROR).size()); + assertThat(memoryAppender.search(CUSTOM_MESSAGE, Level.ERROR)).hasSize(1); } @ParameterizedTest(name = "testSpinnakerHttpException status = {0}") @@ -106,15 +106,15 @@ void testSpinnakerHttpException(int status) throws Exception { URI uri = getUri("/spinnakerHttpException/" + String.valueOf(status)); ResponseEntity entity = restTemplate.getForEntity(uri, String.class); - assertEquals(status, entity.getStatusCode().value()); + assertThat(entity.getStatusCode().value()).isEqualTo(status); // Only expect error logging for a server error, debug otherwise. No need // to fill up logs with client errors assuming the server is doing the best // it can. - assertEquals( - 1, - memoryAppender.countEventsForLevel( - HttpStatus.resolve(status).is5xxServerError() ? Level.ERROR : Level.DEBUG)); + assertThat( + memoryAppender.countEventsForLevel( + HttpStatus.resolve(status).is5xxServerError() ? Level.ERROR : Level.DEBUG)) + .isEqualTo(1); } @ParameterizedTest(name = "testChainedSpinnakerHttpException status = {0}") @@ -123,24 +123,22 @@ void testChainedSpinnakerHttpException(int status) throws Exception { URI uri = getUri("/chainedSpinnakerHttpException/" + String.valueOf(status)); ResponseEntity entity = restTemplate.getForEntity(uri, String.class); - assertEquals(status, entity.getStatusCode().value()); + assertThat(entity.getStatusCode().value()).isEqualTo(status); // Only expect error logging for a server error, debug otherwise. No need // to fill up logs with client errors assuming the server is doing the best // it can. - assertEquals( - 1, - memoryAppender.countEventsForLevel( - HttpStatus.resolve(status).is5xxServerError() ? Level.ERROR : Level.DEBUG)); + assertThat( + memoryAppender.countEventsForLevel( + HttpStatus.resolve(status).is5xxServerError() ? Level.ERROR : Level.DEBUG)) + .isEqualTo(1); // Make sure the message is what we expect. - assertEquals( - 1, - memoryAppender - .search( + assertThat( + memoryAppender.search( CUSTOM_MESSAGE, - HttpStatus.resolve(status).is5xxServerError() ? Level.ERROR : Level.DEBUG) - .size()); + HttpStatus.resolve(status).is5xxServerError() ? Level.ERROR : Level.DEBUG)) + .hasSize(1); } private URI getUri(String path) { diff --git a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java index 1292eed0a..2d24ec97c 100644 --- a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java +++ b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java @@ -16,55 +16,109 @@ package com.netflix.spinnaker.kork.retrofit.exceptions; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; import com.google.gson.Gson; import com.netflix.spinnaker.kork.exceptions.SpinnakerException; import java.io.IOException; import java.util.List; +import okhttp3.Request; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpMethod; import retrofit.RetrofitError; import retrofit.client.Response; import retrofit.converter.ConversionException; import retrofit.converter.GsonConverter; import retrofit.mime.TypedByteArray; -public class SpinnakerServerExceptionTest { +class SpinnakerServerExceptionTest { private static final String CUSTOM_MESSAGE = "custom message"; @Test - public void testSpinnakerNetworkException_NewInstance() { + void testSpinnakerNetworkExceptionWithUrl() { + Throwable cause = new Throwable("arbitrary message"); + String url = "http://some-url/"; + Request request = new Request.Builder().url(url).build(); + SpinnakerNetworkException spinnakerNetworkException = + new SpinnakerNetworkException(cause, request); + assertThat(spinnakerNetworkException.getUrl()).isEqualTo(url); + assertThat(spinnakerNetworkException.getHttpMethod()).isEqualTo(HttpMethod.GET.toString()); + } + + @Test + void testSpinnakerNetworkExceptionWithSpecificMethod() { + Throwable cause = new Throwable("arbitrary message"); + String url = "http://some-url/"; + Request request = + new Request.Builder().url(url).method(HttpMethod.DELETE.toString(), null).build(); + SpinnakerNetworkException spinnakerNetworkException = + new SpinnakerNetworkException(cause, request); + assertThat(spinnakerNetworkException.getUrl()).isEqualTo(url); + assertThat(spinnakerNetworkException.getHttpMethod()).isEqualTo(HttpMethod.DELETE.toString()); + } + + @Test + void testSpinnakerNetworkException_NewInstance() { IOException initialException = new IOException("message"); + String url = "http://localhost"; try { - RetrofitError error = RetrofitError.networkError("http://localhost", initialException); + RetrofitError error = RetrofitError.networkError(url, initialException); throw new SpinnakerNetworkException(error); } catch (SpinnakerException e) { SpinnakerException newException = e.newInstance(CUSTOM_MESSAGE); - assertTrue(newException instanceof SpinnakerNetworkException); - assertEquals(CUSTOM_MESSAGE, newException.getMessage()); - assertEquals(e, newException.getCause()); + assertThat(newException).isInstanceOf(SpinnakerNetworkException.class); + assertThat(newException).hasMessage(CUSTOM_MESSAGE); + assertThat(newException).hasCause(e); + SpinnakerNetworkException spinnakerNetworkException = + (SpinnakerNetworkException) newException; + assertThat(spinnakerNetworkException.getUrl()).isEqualTo(url); } } @Test - public void testSpinnakerServerException_NewInstance() { + void testSpinnakerServerExceptionWithUrl() { + Throwable cause = new Throwable("arbitrary message"); + String url = "http://some-url/"; + Request request = new Request.Builder().url(url).build(); + SpinnakerServerException spinnakerServerException = + new SpinnakerServerException(cause, request); + assertThat(spinnakerServerException.getUrl()).isEqualTo(url); + assertThat(spinnakerServerException.getHttpMethod()).isEqualTo(HttpMethod.GET.toString()); + } + + @Test + void testSpinnakerServerExceptionWithSpecificMethod() { + Throwable cause = new Throwable("arbitrary message"); + String url = "http://some-url/"; + Request request = + new Request.Builder().url(url).method(HttpMethod.DELETE.toString(), null).build(); + SpinnakerServerException spinnakerServerException = + new SpinnakerServerException(cause, request); + assertThat(spinnakerServerException.getUrl()).isEqualTo(url); + assertThat(spinnakerServerException.getHttpMethod()).isEqualTo(HttpMethod.DELETE.toString()); + } + + @Test + void testSpinnakerServerException_NewInstance() { Throwable cause = new Throwable("message"); + String url = "http://localhost"; try { - RetrofitError error = RetrofitError.unexpectedError("http://localhost", cause); + RetrofitError error = RetrofitError.unexpectedError(url, cause); throw new SpinnakerServerException(error); } catch (SpinnakerException e) { SpinnakerException newException = e.newInstance(CUSTOM_MESSAGE); - assertTrue(newException instanceof SpinnakerServerException); - assertEquals(CUSTOM_MESSAGE, newException.getMessage()); - assertEquals(e, newException.getCause()); + assertThat(newException).isInstanceOf(SpinnakerServerException.class); + assertThat(newException).hasMessage(CUSTOM_MESSAGE); + assertThat(newException).hasCause(e); + SpinnakerServerException spinnakerServerException = (SpinnakerServerException) newException; + assertThat(spinnakerServerException.getUrl()).isEqualTo(url); } } @Test - public void testSpinnakerConversionException_NewInstance() { + void testSpinnakerConversionException_NewInstance() { String url = "http://localhost"; String reason = "reason"; @@ -82,13 +136,18 @@ public void testSpinnakerConversionException_NewInstance() { RetrofitError retrofitError = RetrofitError.conversionError( url, response, new GsonConverter(new Gson()), null, conversionException); - throw new SpinnakerConversionException(retrofitError.getMessage(), retrofitError.getCause()); + throw new SpinnakerConversionException(retrofitError); } catch (SpinnakerException e) { SpinnakerException newException = e.newInstance(CUSTOM_MESSAGE); - assertTrue(newException instanceof SpinnakerConversionException); - assertEquals(CUSTOM_MESSAGE, newException.getMessage()); - assertEquals(e, newException.getCause()); + assertThat(newException).isInstanceOf(SpinnakerConversionException.class); + assertThat(newException).hasMessage(CUSTOM_MESSAGE); + assertThat(newException).hasCause(e); + SpinnakerConversionException spinnakerConversionException = + (SpinnakerConversionException) newException; + assertThat(spinnakerConversionException.getRetryable()).isNotNull(); + assertThat(spinnakerConversionException.getRetryable()).isFalse(); + assertThat(spinnakerConversionException.getUrl()).isEqualTo(url); } } } diff --git a/kork-runtime/kork-runtime.gradle b/kork-runtime/kork-runtime.gradle index a8fd2d6af..dba679ede 100644 --- a/kork-runtime/kork-runtime.gradle +++ b/kork-runtime/kork-runtime.gradle @@ -1,4 +1,6 @@ dependencies { + runtimeOnly(platform(project(":spinnaker-dependencies"))) + // Add each included runtime project as a runtime dependency gradle.includedRuntimeProjects.each { runtimeOnly project(it) diff --git a/kork-secrets/src/main/java/com/netflix/spinnaker/kork/secrets/SecretAwarePropertySource.java b/kork-secrets/src/main/java/com/netflix/spinnaker/kork/secrets/SecretAwarePropertySource.java index 926058ae0..23c0b4053 100644 --- a/kork-secrets/src/main/java/com/netflix/spinnaker/kork/secrets/SecretAwarePropertySource.java +++ b/kork-secrets/src/main/java/com/netflix/spinnaker/kork/secrets/SecretAwarePropertySource.java @@ -53,4 +53,8 @@ public String[] getPropertyNames() { public boolean containsProperty(String name) { return delegate.containsProperty(name); } + + public EnumerablePropertySource getDelegate() { + return delegate; + } } diff --git a/kork-security/src/main/java/com/netflix/spinnaker/security/AbstractPermissionEvaluator.java b/kork-security/src/main/java/com/netflix/spinnaker/security/AbstractPermissionEvaluator.java new file mode 100644 index 000000000..905622ad3 --- /dev/null +++ b/kork-security/src/main/java/com/netflix/spinnaker/security/AbstractPermissionEvaluator.java @@ -0,0 +1,79 @@ +/* + * Copyright 2023 Apple, Inc. + * + * 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. + * + */ + +package com.netflix.spinnaker.security; + +import java.io.Serializable; +import org.springframework.security.access.PermissionEvaluator; +import org.springframework.security.core.Authentication; + +/** + * Base implementation for permission evaluators that support {@link AccessControlled} domain + * objects. + */ +public abstract class AbstractPermissionEvaluator implements PermissionEvaluator { + + @Override + public boolean hasPermission( + Authentication authentication, Object targetDomainObject, Object permission) { + if (isDisabled()) { + return true; + } + if (authentication == null || targetDomainObject == null) { + return false; + } + if (SpinnakerAuthorities.isAdmin(authentication)) { + return true; + } + if (targetDomainObject instanceof AccessControlled) { + return ((AccessControlled) targetDomainObject).isAuthorized(authentication, permission); + } + return false; + } + + @Override + public boolean hasPermission( + Authentication authentication, Serializable targetId, String targetType, Object permission) { + if (isDisabled()) { + return true; + } + return hasPermission( + SpinnakerUsers.getUserId(authentication), targetId, targetType, permission); + } + + /** + * Indicates whether permission evaluation is disabled. When this is true, {@code hasPermission} + * calls should return true. This should be overridden to allow for toggling this evaluator at + * runtime. + */ + protected boolean isDisabled() { + return false; + } + + /** + * Alternative method for evaluating a permission where only the identifier of the user and target + * object is available, rather than the authenticated user and target objects themselves. + * + * @param username identifier for user to check permissions for + * @param targetId identifier of the target resource to check permissions + * @param targetType the type of the target resource being checked + * @param permission the permission being validated + * @return true if the permission is granted + */ + public abstract boolean hasPermission( + String username, Serializable targetId, String targetType, Object permission); +} diff --git a/kork-security/src/main/java/com/netflix/spinnaker/security/AccessControlled.java b/kork-security/src/main/java/com/netflix/spinnaker/security/AccessControlled.java index a01d8b5fd..58539de0c 100644 --- a/kork-security/src/main/java/com/netflix/spinnaker/security/AccessControlled.java +++ b/kork-security/src/main/java/com/netflix/spinnaker/security/AccessControlled.java @@ -16,18 +16,26 @@ package com.netflix.spinnaker.security; +import java.util.Collection; import org.springframework.security.core.Authentication; /** * An AccessControlled object is an object that knows its own permissions and can check them against * a given user and authorization. This allows resources to support access control checks via Spring * Security against the resource object directly. + * + * @see AbstractPermissionEvaluator */ public interface AccessControlled { /** * Checks if the authenticated user has a particular authorization on this object. Note that * checking if the user is an admin should be performed by a {@link - * org.springframework.security.access.PermissionEvaluator} rather than in these domain objects. + * org.springframework.security.access.PermissionEvaluator} or by checking {@link + * SpinnakerAuthorities#isAdmin(Authentication)} rather than via this method as the admin role is + * a Spinnaker-specific role. + * + * @see Authorization + * @see SpinnakerAuthorities#hasAnyRole(Authentication, Collection) */ boolean isAuthorized(Authentication authentication, Object authorization); } diff --git a/kork-security/src/main/java/com/netflix/spinnaker/security/Authorization.java b/kork-security/src/main/java/com/netflix/spinnaker/security/Authorization.java new file mode 100644 index 000000000..4f2512068 --- /dev/null +++ b/kork-security/src/main/java/com/netflix/spinnaker/security/Authorization.java @@ -0,0 +1,45 @@ +/* + * Copyright 2022 Apple Inc. + * + * 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. + */ + +package com.netflix.spinnaker.security; + +import java.util.Locale; +import javax.annotation.Nullable; +import org.springframework.security.core.Authentication; + +/** + * Defines types of authorizations supported by {@link AccessControlled#isAuthorized(Authentication, + * Object)}. + */ +public enum Authorization { + READ, + WRITE, + EXECUTE, + CREATE, + ; + + public static @Nullable Authorization parse(@Nullable Object o) { + if (o == null) { + return null; + } + String name = o.toString().toUpperCase(Locale.ROOT); + try { + return valueOf(name); + } catch (IllegalArgumentException e) { + return null; + } + } +} diff --git a/kork-security/src/main/java/com/netflix/spinnaker/security/AuthorizationMapControlled.java b/kork-security/src/main/java/com/netflix/spinnaker/security/AuthorizationMapControlled.java new file mode 100644 index 000000000..d13ad4f0a --- /dev/null +++ b/kork-security/src/main/java/com/netflix/spinnaker/security/AuthorizationMapControlled.java @@ -0,0 +1,32 @@ +/* + * Copyright 2023 Apple, Inc. + * + * 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. + * + */ + +package com.netflix.spinnaker.security; + +import javax.annotation.Nullable; + +/** + * Common interface for access-controlled classes which use a permission map of {@link + * Authorization} enums. + */ +public interface AuthorizationMapControlled extends PermissionMapControlled { + @Nullable + @Override + default Authorization valueOf(@Nullable Object authorization) { + return Authorization.parse(authorization); + } +} diff --git a/kork-security/src/main/java/com/netflix/spinnaker/security/PermissionMapControlled.java b/kork-security/src/main/java/com/netflix/spinnaker/security/PermissionMapControlled.java new file mode 100644 index 000000000..4abe8a391 --- /dev/null +++ b/kork-security/src/main/java/com/netflix/spinnaker/security/PermissionMapControlled.java @@ -0,0 +1,52 @@ +/* + * Copyright 2023 Apple Inc. + * + * 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. + */ + +package com.netflix.spinnaker.security; + +import com.netflix.spinnaker.kork.annotations.Alpha; +import java.util.Map; +import java.util.Set; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import org.springframework.security.core.Authentication; + +/** + * Common interface for access-controlled classes which use a permission map. + * + * @param Authorization enum type + */ +@Alpha +public interface PermissionMapControlled> + extends AccessControlled { + @Nullable + Authorization valueOf(@Nullable Object authorization); + + @Nonnull + default Map> getPermissions() { + return Map.of(); + } + + @Override + default boolean isAuthorized(Authentication authentication, Object authorization) { + Authorization auth = valueOf(authorization); + if (auth == null) { + return false; + } + Set permittedRoles = getPermissions().getOrDefault(auth, Set.of()); + return permittedRoles.isEmpty() + || SpinnakerAuthorities.hasAnyRole(authentication, permittedRoles); + } +} diff --git a/kork-security/src/main/java/com/netflix/spinnaker/security/SpinnakerAuthorities.java b/kork-security/src/main/java/com/netflix/spinnaker/security/SpinnakerAuthorities.java new file mode 100644 index 000000000..75fba07d4 --- /dev/null +++ b/kork-security/src/main/java/com/netflix/spinnaker/security/SpinnakerAuthorities.java @@ -0,0 +1,90 @@ +/* + * Copyright 2022 Apple, Inc. + * + * 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. + */ + +package com.netflix.spinnaker.security; + +import java.util.Collection; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.SimpleGrantedAuthority; + +/** + * Constants and utilities for working with Spring Security GrantedAuthority objects specific to + * Spinnaker and Fiat. Spinnaker-specific roles are represented here as granted authorities with the + * {@code SPINNAKER_} prefix. + */ +public class SpinnakerAuthorities { + private static final String ROLE_PREFIX = "ROLE_"; + + public static final String ADMIN = "SPINNAKER_ADMIN"; + /** Granted authority for Spinnaker administrators. */ + public static final GrantedAuthority ADMIN_AUTHORITY = new SimpleGrantedAuthority(ADMIN); + + /** Granted authority for anonymous users. */ + public static final GrantedAuthority ANONYMOUS_AUTHORITY = forRoleName("ANONYMOUS"); + + /** Creates a granted authority corresponding to the provided name of a role. */ + @Nonnull + public static GrantedAuthority forRoleName(@Nonnull String role) { + return new SimpleGrantedAuthority(ROLE_PREFIX + role); + } + + /** Checks if the given user is a Spinnaker admin. */ + public static boolean isAdmin(@Nullable Authentication authentication) { + return authentication != null + && authentication.getAuthorities().contains(SpinnakerAuthorities.ADMIN_AUTHORITY); + } + + /** Checks if the given user has the provided role. */ + public static boolean hasRole(@Nullable Authentication authentication, @Nonnull String role) { + return authentication != null && streamRoles(authentication).anyMatch(role::equals); + } + + /** Checks if the given user has any of the provided roles. */ + public static boolean hasAnyRole( + @Nullable Authentication authentication, @Nonnull Collection roles) { + return authentication != null && streamRoles(authentication).anyMatch(roles::contains); + } + + /** Gets the list of roles assigned to the given user. */ + @Nonnull + public static List getRoles(@Nullable Authentication authentication) { + if (authentication == null) { + return List.of(); + } + return streamRoles(authentication).distinct().collect(Collectors.toList()); + } + + @Nonnull + private static Stream streamRoles(@Nonnull Authentication authentication) { + return authentication.getAuthorities().stream() + .filter(SpinnakerAuthorities::isRole) + .map(SpinnakerAuthorities::getRole); + } + + private static boolean isRole(@Nonnull GrantedAuthority authority) { + return authority.getAuthority().startsWith(ROLE_PREFIX); + } + + private static String getRole(@Nonnull GrantedAuthority authority) { + return authority.getAuthority().substring(ROLE_PREFIX.length()); + } +} diff --git a/kork-security/src/main/java/com/netflix/spinnaker/security/SpinnakerUsers.java b/kork-security/src/main/java/com/netflix/spinnaker/security/SpinnakerUsers.java new file mode 100644 index 000000000..15c799465 --- /dev/null +++ b/kork-security/src/main/java/com/netflix/spinnaker/security/SpinnakerUsers.java @@ -0,0 +1,45 @@ +/* + * Copyright 2023 Apple, Inc. + * + * 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. + * + */ + +package com.netflix.spinnaker.security; + +import com.netflix.spinnaker.kork.annotations.NonnullByDefault; +import javax.annotation.Nullable; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; + +/** Constants and utilities related to Spinnaker users (AKA principals). */ +@NonnullByDefault +public class SpinnakerUsers { + /** String constant for the anonymous userid. */ + public static final String ANONYMOUS = "anonymous"; + + /** Gets the userid of the provided authentication token. */ + public static String getUserId(@Nullable Authentication authentication) { + return authentication != null ? authentication.getName() : ANONYMOUS; + } + + /** Gets the current Spinnaker userid. */ + public static String getCurrentUserId() { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + if (authentication != null) { + return getUserId(authentication); + } + // fall back to request header context if relevant (AuthenticatedRequestFilter) + return AuthenticatedRequest.getSpinnakerUser().orElse(ANONYMOUS); + } +} diff --git a/kork-security/src/test/groovy/com/netflix/spinnaker/security/AccessControlledSpec.groovy b/kork-security/src/test/groovy/com/netflix/spinnaker/security/AccessControlledSpec.groovy new file mode 100644 index 000000000..d0845bff9 --- /dev/null +++ b/kork-security/src/test/groovy/com/netflix/spinnaker/security/AccessControlledSpec.groovy @@ -0,0 +1,67 @@ +/* + * Copyright 2023 Apple Inc. + * + * 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. + */ + +package com.netflix.spinnaker.security + +import org.springframework.security.authentication.TestingAuthenticationToken +import org.springframework.security.core.AuthenticatedPrincipal +import spock.lang.Specification + +import static groovy.lang.Tuple.tuple + +class AccessControlledSpec extends Specification { + static class TestCredentials implements AuthorizationMapControlled { + Map> permissions + } + + static class TestPermissionEvaluator extends AbstractPermissionEvaluator { + Map, Map>> targetIdTypesToUsernamePermissions = [:] + + @Override + boolean hasPermission(String username, Serializable targetId, String targetType, Object permission) { + def targetKey = tuple(targetId, targetType) + def usernamePermissions = targetIdTypesToUsernamePermissions[targetKey] + def permissions = usernamePermissions?[username] + permissions != null && permission in permissions + } + } + + static class TestUser implements AuthenticatedPrincipal { + String name + } + + void "check hasPermission for expected permissions"() { + when: + def username = 'alpha' + def user = new TestUser(name: username) + def role = 'dev' + def authentication = new TestingAuthenticationToken(user, null, [SpinnakerAuthorities.forRoleName(role)]) + def credentials = new TestCredentials(permissions: Map.of(Authorization.READ, Set.of(role), Authorization.WRITE, Set.of('ops'))) + def permissionEvaluator = new TestPermissionEvaluator() + then: + permissionEvaluator.hasPermission(authentication, credentials, Authorization.READ) + !permissionEvaluator.hasPermission(authentication, credentials, Authorization.WRITE) + !permissionEvaluator.hasPermission(null, credentials, Authorization.READ) + !permissionEvaluator.hasPermission(authentication, null, Authorization.READ) + when: + def targetId = 14 + def targetType = 'entry' + permissionEvaluator.targetIdTypesToUsernamePermissions[tuple(targetId, targetType)] = Map.of(username, Set.of(Authorization.READ)) + then: + permissionEvaluator.hasPermission(authentication, targetId, targetType, Authorization.READ) + permissionEvaluator.hasPermission(username, targetId, targetType, Authorization.READ) + } +} diff --git a/kork-security/src/test/groovy/com/netflix/spinnaker/security/AuthorizationSpec.groovy b/kork-security/src/test/groovy/com/netflix/spinnaker/security/AuthorizationSpec.groovy new file mode 100644 index 000000000..7247df46f --- /dev/null +++ b/kork-security/src/test/groovy/com/netflix/spinnaker/security/AuthorizationSpec.groovy @@ -0,0 +1,39 @@ +/* + * Copyright 2023 Apple Inc. + * + * 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. + */ + +package com.netflix.spinnaker.security + +import spock.lang.Specification + +class AuthorizationSpec extends Specification { + void "parse returns appropriate value"() { + expect: + Authorization.parse(input) == output + where: + input || output + null || null + 'read' || Authorization.READ + 'READ' || Authorization.READ + "Read" || Authorization.READ + Authorization.READ || Authorization.READ + 'write' || Authorization.WRITE + "${Authorization.EXECUTE}" || Authorization.EXECUTE + 'create' || Authorization.CREATE + 'unsupported' || null + ' read ' || null + 'query' || null + } +} diff --git a/kork-security/src/test/groovy/com/netflix/spinnaker/security/SpinnakerAuthoritiesSpec.groovy b/kork-security/src/test/groovy/com/netflix/spinnaker/security/SpinnakerAuthoritiesSpec.groovy new file mode 100644 index 000000000..c3dbe8ca3 --- /dev/null +++ b/kork-security/src/test/groovy/com/netflix/spinnaker/security/SpinnakerAuthoritiesSpec.groovy @@ -0,0 +1,63 @@ +/* + * Copyright 2023 Apple Inc. + * + * 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. + */ + +package com.netflix.spinnaker.security + +import org.springframework.security.authentication.TestingAuthenticationToken +import org.springframework.security.core.Authentication +import org.springframework.security.core.GrantedAuthority +import spock.lang.Specification + +class SpinnakerAuthoritiesSpec extends Specification { + void "isAdmin returns expected value"() { + expect: + SpinnakerAuthorities.isAdmin(authentication) == isAdmin + where: + authentication || isAdmin + null || false + u(SpinnakerAuthorities.ADMIN_AUTHORITY) || true + u(SpinnakerAuthorities.ANONYMOUS_AUTHORITY) || false + } + + void "hasRole returns expected value"() { + expect: + SpinnakerAuthorities.hasRole(authentication, 'dev') == hasRole + where: + authentication || hasRole + null || false + u('ROLE_dev') || true + u(SpinnakerAuthorities.forRoleName('dev')) || true + u(SpinnakerAuthorities.ANONYMOUS_AUTHORITY) || false + } + + void "getRoles returns expected value"() { + expect: + SpinnakerAuthorities.getRoles(authentication) == expectedRoles + where: + authentication || expectedRoles + null || [] + u('ROLE_a', 'ROLE_b') || ['a', 'b'] + u(SpinnakerAuthorities.forRoleName('c')) || ['c'] + } + + private static Authentication u(String... authorities) { + new TestingAuthenticationToken(null, null, authorities) + } + + private static Authentication u(GrantedAuthority... authorities) { + new TestingAuthenticationToken(null, null, List.of(authorities)) + } +} diff --git a/kork-security/src/test/groovy/com/netflix/spinnaker/security/SpinnakerUsersSpec.groovy b/kork-security/src/test/groovy/com/netflix/spinnaker/security/SpinnakerUsersSpec.groovy new file mode 100644 index 000000000..0b869548e --- /dev/null +++ b/kork-security/src/test/groovy/com/netflix/spinnaker/security/SpinnakerUsersSpec.groovy @@ -0,0 +1,96 @@ +/* + * Copyright 2023 Apple Inc. + * + * 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. + */ + +package com.netflix.spinnaker.security + +import org.springframework.security.authentication.TestingAuthenticationToken +import org.springframework.security.core.AuthenticatedPrincipal +import org.springframework.security.core.context.SecurityContextHolder +import org.springframework.security.core.userdetails.User +import spock.lang.Specification + +import java.security.Principal + +class SpinnakerUsersSpec extends Specification { + void "user id of null authentication returns anonymous"() { + when: + def authentication = null + then: + SpinnakerUsers.getUserId(authentication) == SpinnakerUsers.ANONYMOUS + } + + void "user id of UserDetails returns username"() { + when: + def username = 'alpha' + def user = User.withUsername(username).password('').authorities('ROLE_USER').build() + def authentication = new TestingAuthenticationToken(user, null) + then: + SpinnakerUsers.getUserId(authentication) == username + } + + void "user id of AuthenticatedPrincipal returns name"() { + when: + def username = 'beta' + def principal = new TestAuthenticatedPrincipal(name: username) + def authentication = new TestingAuthenticationToken(principal, null) + then: + SpinnakerUsers.getUserId(authentication) == username + } + + void "user id of Principal returns name"() { + when: + def username = 'gamma' + def principal = new TestPrincipal(name: username) + def authentication = new TestingAuthenticationToken(principal, null) + then: + SpinnakerUsers.getUserId(authentication) == username + } + + void "current user id is anonymous by default"() { + when: + SecurityContextHolder.context.authentication = null + AuthenticatedRequest.clear() + then: + SpinnakerUsers.currentUserId == SpinnakerUsers.ANONYMOUS + } + + void "current user id uses current security context first"() { + when: + def username = 'delta' + def principal = new TestPrincipal(name: username) + SecurityContextHolder.context.authentication = new TestingAuthenticationToken(principal, null) + AuthenticatedRequest.setUser("not $username") + then: + SpinnakerUsers.currentUserId == username + } + + void "current user id uses authenticated request second"() { + when: + def username = 'epsilon' + SecurityContextHolder.clearContext() + AuthenticatedRequest.setUser(username) + then: + SpinnakerUsers.currentUserId == username + } + + static class TestPrincipal implements Principal { + String name + } + + static class TestAuthenticatedPrincipal implements AuthenticatedPrincipal { + String name + } +} diff --git a/kork-sql-test/src/main/java/com/netflix/spinnaker/kork/sql/test/SqlTestUtil.java b/kork-sql-test/src/main/java/com/netflix/spinnaker/kork/sql/test/SqlTestUtil.java index 1e55f132f..2ee16db68 100644 --- a/kork-sql-test/src/main/java/com/netflix/spinnaker/kork/sql/test/SqlTestUtil.java +++ b/kork-sql-test/src/main/java/com/netflix/spinnaker/kork/sql/test/SqlTestUtil.java @@ -223,7 +223,7 @@ public static TestDatabase initDatabase( Liquibase migrate; try { - DatabaseChangeLog changeLog = new DatabaseChangeLog(); + DatabaseChangeLog changeLog = new DatabaseChangeLog("db/changelog/"); changeLog.setChangeLogParameters( new ChangeLogParameters( diff --git a/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/SqlMigrationProperties.kt b/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/SqlMigrationProperties.kt index 7a52c2025..e88a7c578 100644 --- a/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/SqlMigrationProperties.kt +++ b/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/SqlMigrationProperties.kt @@ -15,6 +15,8 @@ */ package com.netflix.spinnaker.kork.sql.config +import liquibase.GlobalConfiguration + /** * Defines the configuration properties for connecting to a SQL database for schema migration purposes. * @@ -23,11 +25,14 @@ package com.netflix.spinnaker.kork.sql.config * @param password The password to authenticate the [user] * @param driver The JDBC driver name * @param additionalChangeLogs A list of additional change log paths. This is useful for libraries and extensions. + * @param duplicateFileMode flag to handle if multiple files are found in the search path that have duplicate paths. */ data class SqlMigrationProperties( var jdbcUrl: String? = null, var user: String? = null, var password: String? = null, var driver: String? = null, - var additionalChangeLogs: List = mutableListOf() + var additionalChangeLogs: List = mutableListOf(), + var duplicateFileMode: GlobalConfiguration.DuplicateFileMode = GlobalConfiguration.DUPLICATE_FILE_MODE.defaultValue + ) diff --git a/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/migration/SpringLiquibaseProxy.kt b/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/migration/SpringLiquibaseProxy.kt index 3cf1a7935..574ceb708 100644 --- a/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/migration/SpringLiquibaseProxy.kt +++ b/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/migration/SpringLiquibaseProxy.kt @@ -16,6 +16,8 @@ package com.netflix.spinnaker.kork.sql.migration import com.netflix.spinnaker.kork.sql.config.SqlMigrationProperties +import liquibase.GlobalConfiguration +import liquibase.Scope import javax.sql.DataSource import liquibase.integration.spring.SpringLiquibase import org.springframework.jdbc.datasource.SingleConnectionDataSource @@ -52,18 +54,20 @@ class SpringLiquibaseProxy( super.afterPropertiesSet() // Then if anything else has been defined, do that afterwards - (sqlMigrationProperties.additionalChangeLogs + korkAdditionalChangelogs) - .map { - SpringLiquibase().apply { - changeLog = "classpath:$it" - dataSource = createDataSource() - resourceLoader = this@SpringLiquibaseProxy.resourceLoader - shouldRun = !sqlReadOnly + Scope.child(GlobalConfiguration.DUPLICATE_FILE_MODE.key, sqlMigrationProperties.duplicateFileMode) { + (sqlMigrationProperties.additionalChangeLogs + korkAdditionalChangelogs) + .map { + SpringLiquibase().apply { + changeLog = "classpath:$it" + dataSource = createDataSource() + resourceLoader = this@SpringLiquibaseProxy.resourceLoader + shouldRun = !sqlReadOnly + } } - } - .forEach { - it.afterPropertiesSet() - } + .forEach { + it.afterPropertiesSet() + } + } } private fun createDataSource(): DataSource = diff --git a/kork-sql/src/test/kotlin/com/netflix/spinnaker/kork/sql/SpringStartupTests.kt b/kork-sql/src/test/kotlin/com/netflix/spinnaker/kork/sql/SpringStartupTests.kt index 8310f98da..d23df51ee 100644 --- a/kork-sql/src/test/kotlin/com/netflix/spinnaker/kork/sql/SpringStartupTests.kt +++ b/kork-sql/src/test/kotlin/com/netflix/spinnaker/kork/sql/SpringStartupTests.kt @@ -30,6 +30,7 @@ import org.springframework.boot.autoconfigure.SpringBootApplication import org.springframework.boot.test.context.SpringBootTest import org.springframework.context.ApplicationContext import org.springframework.context.annotation.Import +import org.springframework.test.context.TestPropertySource import org.springframework.test.context.junit.jupiter.SpringExtension import strikt.api.expectThat import strikt.assertions.isA @@ -43,6 +44,7 @@ import strikt.assertions.isNotNull "sql.enabled=true", "sql.migration.jdbcUrl=jdbc:h2:mem:test", "sql.migration.dialect=H2", + "sql.migration.duplicateFileMode=WARN", "sql.connectionPool.jdbcUrl=jdbc:h2:mem:test", "sql.connectionPool.dialect=H2" ] diff --git a/kork-sql/src/test/resources/application-test.yml b/kork-sql/src/test/resources/application-test.yml index c3b01d241..662c6420d 100644 --- a/kork-sql/src/test/resources/application-test.yml +++ b/kork-sql/src/test/resources/application-test.yml @@ -26,10 +26,12 @@ sql: jdbcUrl: "jdbc:h2:mem:test" user: password: + duplicateFileMode: WARN secondaryMigration: jdbcUrl: "jdbc:h2:mem:test" user: password: + duplicateFileMode: WARN --- spring: @@ -52,3 +54,4 @@ sql: jdbcUrl: "jdbc:h2:mem:test" user: password: + duplicateFileMode: WARN diff --git a/kork-sql/src/test/resources/db/changelog-master.yml b/kork-sql/src/test/resources/db/changelog-master.yml index daf9f00fc..3d2c63d3b 100644 --- a/kork-sql/src/test/resources/db/changelog-master.yml +++ b/kork-sql/src/test/resources/db/changelog-master.yml @@ -1 +1,21 @@ -databaseChangeLog: [] +databaseChangeLog: + - changeSet: + id: create-sample-table + author: kirangodishala + changes: + - createTable: + tableName: sample + columns: + - column: + name: id + type: boolean + constraints: + primaryKey: true + nullable: false + - modifySql: + dbms: mysql + append: + value: " engine innodb" + rollback: + - dropTable: + tableName: sample diff --git a/kork-swagger/kork-swagger.gradle b/kork-swagger/kork-swagger.gradle index 0ba8870b2..5d57e7d5c 100644 --- a/kork-swagger/kork-swagger.gradle +++ b/kork-swagger/kork-swagger.gradle @@ -7,7 +7,6 @@ dependencies { implementation "com.google.guava:guava" implementation "org.springframework.boot:spring-boot-autoconfigure" - implementation "io.springfox:springfox-swagger2" - implementation "io.springfox:springfox-swagger-ui" + implementation "io.springfox:springfox-boot-starter" } diff --git a/kork-swagger/src/main/java/com/netflix/spinnaker/config/SwaggerConfig.java b/kork-swagger/src/main/java/com/netflix/spinnaker/config/SwaggerConfig.java index 54823325c..662e45b2f 100644 --- a/kork-swagger/src/main/java/com/netflix/spinnaker/config/SwaggerConfig.java +++ b/kork-swagger/src/main/java/com/netflix/spinnaker/config/SwaggerConfig.java @@ -16,13 +16,10 @@ package com.netflix.spinnaker.config; -import static com.google.common.base.Predicates.or; - -import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import java.util.List; import java.util.Objects; -import java.util.stream.Collectors; +import java.util.function.Predicate; import javax.annotation.Nullable; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.ConfigurationProperties; @@ -32,11 +29,9 @@ import springfox.documentation.builders.RequestHandlerSelectors; import springfox.documentation.service.ApiInfo; import springfox.documentation.spi.DocumentationType; -import springfox.documentation.spring.web.paths.AbstractPathProvider; +import springfox.documentation.spring.web.paths.DefaultPathProvider; import springfox.documentation.spring.web.plugins.Docket; -import springfox.documentation.swagger2.annotations.EnableSwagger2; -@EnableSwagger2 @Configuration @ConditionalOnProperty("swagger.enabled") @ConfigurationProperties(prefix = "swagger") @@ -54,7 +49,7 @@ public class SwaggerConfig { @Bean public Docket gateApi() { return new Docket(DocumentationType.SWAGGER_2) - .pathProvider(new BasePathProvider(basePath, documentationPath)) + .pathProvider(new BasePathProvider(documentationPath)) .select() .apis(RequestHandlerSelectors.any()) .paths(paths()) @@ -80,7 +75,7 @@ private static Class getClassIfPresent(String name) { } private Predicate paths() { - return or(patterns.stream().map(PathSelectors::regex).collect(Collectors.toList())); + return patterns.stream().map(PathSelectors::regex).reduce((x, y) -> x.or(y)).get(); } private ApiInfo apiInfo() { @@ -123,20 +118,13 @@ public String getDocumentationPath() { return documentationPath; } - public class BasePathProvider extends AbstractPathProvider { - private String basePath; + public class BasePathProvider extends DefaultPathProvider { private String documentationPath; - private BasePathProvider(String basePath, String documentationPath) { - this.basePath = basePath; + private BasePathProvider(String documentationPath) { this.documentationPath = documentationPath; } - @Override - protected String applicationPath() { - return basePath; - } - @Override protected String getDocumentationPath() { return documentationPath; diff --git a/kork-tomcat/kork-tomcat.gradle b/kork-tomcat/kork-tomcat.gradle index 8dedbdb51..30a305248 100644 --- a/kork-tomcat/kork-tomcat.gradle +++ b/kork-tomcat/kork-tomcat.gradle @@ -8,6 +8,7 @@ dependencies { implementation("com.netflix.spectator:spectator-api") implementation("com.google.guava:guava") implementation(project(":kork-core")) + implementation(project(":kork-crypto")) testImplementation "org.springframework.boot:spring-boot-starter-test" } diff --git a/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfigurationProperties.java b/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfigurationProperties.java index a5eaf94c8..1f890711a 100644 --- a/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfigurationProperties.java +++ b/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfigurationProperties.java @@ -17,6 +17,7 @@ package com.netflix.spinnaker.kork.tomcat; +import com.netflix.spinnaker.kork.crypto.CipherSuites; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -31,27 +32,9 @@ public class TomcatConfigurationProperties { private String relaxedQueryCharacters = ""; private String relaxedPathCharacters = ""; - private List tlsVersions = new ArrayList<>(Arrays.asList("TLSv1.2", "TLSv1.1")); - - // Defaults from https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility - // with some extra ciphers (non SHA384/256) to support TLSv1.1 and some non EC ciphers - private List cipherSuites = - new ArrayList<>( - Arrays.asList( - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384", - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", - "TLS_DHE_RSA_WITH_AES_256_CBC_SHA", - "TLS_DHE_RSA_WITH_AES_128_CBC_SHA")); + private List tlsVersions = new ArrayList<>(Arrays.asList("TLSv1.3", "TLSv1.2")); + + private List cipherSuites = CipherSuites.getRecommendedCiphers(); private Boolean rejectIllegalHeader; diff --git a/kork-web/src/main/groovy/com/netflix/spinnaker/okhttp/OkHttpClientConfigurationProperties.groovy b/kork-web/src/main/groovy/com/netflix/spinnaker/okhttp/OkHttpClientConfigurationProperties.groovy index b69e88b13..12a2390dc 100644 --- a/kork-web/src/main/groovy/com/netflix/spinnaker/okhttp/OkHttpClientConfigurationProperties.groovy +++ b/kork-web/src/main/groovy/com/netflix/spinnaker/okhttp/OkHttpClientConfigurationProperties.groovy @@ -16,6 +16,7 @@ package com.netflix.spinnaker.okhttp +import com.netflix.spinnaker.kork.crypto.CipherSuites import groovy.transform.AutoClone import groovy.transform.Canonical import java.time.Duration @@ -43,23 +44,8 @@ class OkHttpClientConfigurationProperties { String secureRandomInstanceType = "NativePRNGNonBlocking" // TLS1.1 isn't supported in newer JVMs... do NOT try to add back - it's also insecure - List tlsVersions = ["TLSv1.2", "TLSv1.3"] - //Defaults from https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility - // with some extra ciphers (non SHA384/256) to support TLSv1.1 and some non EC ciphers - List cipherSuites = [ - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384", - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA" - ] + List tlsVersions = ["TLSv1.3", "TLSv1.2"] + List cipherSuites = CipherSuites.recommendedCiphers /** * Provide backwards compatibility for 'okHttpClient.connectTimoutMs' diff --git a/kork-web/src/main/java/com/netflix/spinnaker/kork/web/context/RequestContext.java b/kork-web/src/main/java/com/netflix/spinnaker/kork/web/context/RequestContext.java index dc20ec791..1e5750513 100644 --- a/kork-web/src/main/java/com/netflix/spinnaker/kork/web/context/RequestContext.java +++ b/kork-web/src/main/java/com/netflix/spinnaker/kork/web/context/RequestContext.java @@ -46,6 +46,14 @@ default Optional getApplication() { return get(Header.APPLICATION); } + default Optional getAccount() { + return get(Header.ACCOUNT); + } + + default Optional getCloudProvider() { + return get(Header.CLOUD_PROVIDER); + } + default Optional getExecutionType() { return get(Header.EXECUTION_TYPE); } @@ -83,6 +91,14 @@ default void setApplication(String value) { set(Header.APPLICATION, value); } + default void setAccount(String value) { + set(Header.ACCOUNT, value); + } + + default void setCloudProvider(String value) { + set(Header.CLOUD_PROVIDER, value); + } + default void setExecutionType(String value) { set(Header.EXECUTION_TYPE, value); } diff --git a/spinnaker-dependencies/spinnaker-dependencies.gradle b/spinnaker-dependencies/spinnaker-dependencies.gradle index 909a5a54e..d7e265acd 100644 --- a/spinnaker-dependencies/spinnaker-dependencies.gradle +++ b/spinnaker-dependencies/spinnaker-dependencies.gradle @@ -9,35 +9,24 @@ ext { arrow : "0.13.2", aws : "1.12.176", awsv2 : "2.19.0", - bouncycastle : "1.70", + bouncycastle : "1.77", brave : "5.12.3", gcp : "25.3.0", - groovy : "2.5.15", - jooq : "3.13.6", jsch : "0.1.54", jschAgentProxy : "0.0.9", - // spring boot 2.5.14 specifies logback 1.2.11, but a rosco test hung with - // 1.2.11 from https://jira.qos.ch/browse/LOGBACK-1623 so pinning it to 1.2.12 - // until spring boot upgrade 2.5.15 or above. - logback : "1.2.12", protobuf : "3.21.12", okhttp : "2.7.5", // CVE-2016-2402 okhttp3 : "4.9.3", openapi : "1.3.9", // this needs to be kept in sync with spring boot as it pulls in the spring-boot-dependencies BOM - //Spring boot 2.5.14 upgrade brings io.rest-assured 4.3.3 as transitive dependency. - //io.rest-assured 4.3.x require groovy 3.0.2. So, pinning the nearest version - //using groovy 2.x. This can be removed with groovy 3.x upgrade. - //[https://github.com/rest-assured/rest-assured/blob/9b683130c93188cabdef850e89d0c9417d847a17/changelog.txt#L200] - restassured : "4.2.0", retrofit : "1.9.0", retrofit2 : "2.8.1", spectator : "1.0.6", spek : "1.1.5", spek2 : "2.0.9", - springBoot : "2.5.14", + springBoot : "2.5.15", springCloud : "2020.0.6", - springfoxSwagger : "2.9.2", - swagger : "1.5.20", //this should stay in sync with what springfoxSwagger expects + springfoxSwagger : "3.0.0", + swagger : "1.5.20", //this should stay in sync with what springfoxSwagger expects. // Spring boot 2.4.13 brings in 9.0.55. Spring boot 2.5.14 brings in // 9.0.63. Use 9.0.69 to resolve CVE-2022-42252 and CVE-2022-45143. Spring @@ -61,53 +50,26 @@ dependencies { // 2.16.0 is subject to CVE-2021-45105. 2.17.0 is subject to CVE-2021-44832, so use >= 2.17.1. api(platform("org.apache.logging.log4j:log4j-bom:2.20.0")) - //Upgrade of spring boot 2.5.x brings groovy 3.x as transitive dependency. - //To avoid transitive upgrade of groovy, pinning it with enforcedPlatform() closure. - //It forces version for internal submodules of kork as well as for all the consumer spinnaker services. - api(enforcedPlatform("org.codehaus.groovy:groovy-bom")){ - version { - strictly "${versions.groovy}" - } - } - api(platform("org.jetbrains.kotlinx:kotlinx-coroutines-bom:1.4.3")) + api(platform("org.jetbrains.kotlinx:kotlinx-coroutines-bom:1.5.2")) //kotlinVersion comes from gradle.properties since we have kotlin code in // this project and need to configure gradle plugins etc. api(platform("org.jetbrains.kotlin:kotlin-bom:$kotlinVersion")) - api(platform("com.fasterxml.jackson:jackson-bom:2.12.7.20221012")) api(platform("io.zipkin.brave:brave-bom:${versions.brave}")) + api(platform("org.junit:junit-bom:5.8.2")) // remove with spring boot >= 2.6.2 api(platform("org.springframework.boot:spring-boot-dependencies:${versions.springBoot}")) api(platform("com.amazonaws:aws-java-sdk-bom:${versions.aws}")) api(platform("com.google.protobuf:protobuf-bom:${versions.protobuf}")) api(platform("com.google.cloud:libraries-bom:${versions.gcp}")) api(platform("software.amazon.awssdk:bom:${versions.awsv2}")) - api(platform("com.google.cloud:google-cloud-secretmanager:2.1.7")) api(platform("org.springframework.cloud:spring-cloud-dependencies:${versions.springCloud}")) api(platform("io.strikt:strikt-bom:0.31.0")) - api(platform("org.spockframework:spock-bom:1.3-groovy-2.5")) - api(platform("com.oracle.oci.sdk:oci-java-sdk-bom:1.5.17")) + api(platform("org.spockframework:spock-bom:2.0-groovy-3.0")) + api(platform("com.oracle.oci.sdk:oci-java-sdk-bom:3.21.0")) api(platform("org.testcontainers:testcontainers-bom:1.16.2")) api(platform("io.arrow-kt:arrow-stack:${versions.arrow}")) constraints { api("cglib:cglib-nodep:3.3.0") - //A bug is reported in 1.2.11 and fixed in 1.2.12. - //So pinning the version to 1.2.12 untill spring boot upgrade to 2.5.15 or above. - //[https://jira.qos.ch/browse/LOGBACK-1623] - api("ch.qos.logback:logback-core"){ - version { - strictly "${versions.logback}" - } - } - api("ch.qos.logback:logback-classic"){ - version { - strictly "${versions.logback}" - } - } - api("ch.qos.logback:logback-classic"){ - version { - strictly "${versions.logback}" - } - } api("com.amazonaws:aws-java-sdk:${versions.aws}") api("com.google.api-client:google-api-client:1.30.10") // TODO: Track update for CVE-2020-7692, reanalysis pending. api("com.google.apis:google-api-services-admin-directory:directory_v1-rev105-1.25.0") @@ -116,12 +78,13 @@ dependencies { api("com.google.apis:google-api-services-iam:v1-rev267-1.25.0") api("com.google.apis:google-api-services-monitoring:v3-rev477-1.25.0") api("com.google.apis:google-api-services-storage:v1-rev141-1.25.0") + api("com.google.cloud:google-cloud-secretmanager:2.3.10") api("com.google.code.findbugs:jsr305:3.0.2") api("com.google.guava:guava:30.0-jre") // JinJava 2.5.3 has a bad bug: https://github.com/HubSpot/jinjava/issues/429 api("com.hubspot.jinjava:jinjava:2.5.2") api("com.jakewharton.retrofit:retrofit1-okhttp3-client:1.1.0") - api("com.jcraft.jsch:${versions.jsch}") + api("com.jcraft:jsch:${versions.jsch}") api("com.jcraft:jsch.agentproxy.connector-factory:${versions.jschAgentProxy}") api("com.jcraft:jsch.agentproxy.jsch:${versions.jschAgentProxy}") api("com.natpryce:hamkrest:1.4.2.2") @@ -140,7 +103,6 @@ dependencies { api("com.netflix.spectator:spectator-web-spring:${versions.spectator}") api("com.netflix.spectator:spectator-reg-micrometer:${versions.spectator}") api("com.nimbusds:nimbus-jose-jwt:7.9") - api("io.spinnaker.embedded-redis:embedded-redis:0.9.0") api("com.nhaarman.mockitokotlin2:mockito-kotlin:2.2.0") api("com.nhaarman:mockito-kotlin:1.6.0") api("com.ninja-squad:springmockk:2.0.3") @@ -171,37 +133,21 @@ dependencies { api("de.huxhorn.sulky:de.huxhorn.sulky.ulid:8.2.0") api("dev.minutest:minutest:1.13.0") api("io.mockk:mockk:1.10.5") - api("io.rest-assured:rest-assured"){ - version { - strictly "${versions.restassured}" - } - } - api("io.rest-assured:rest-assured-common"){ - version { - strictly "${versions.restassured}" - } - } - api("io.rest-assured:json-path"){ - version { - strictly "${versions.restassured}" - } - } - api("io.rest-assured:xml-path"){ - version { - strictly "${versions.restassured}" - } - } - api("io.springfox:springfox-swagger-ui:${versions.springfoxSwagger}") + api("io.springfox:springfox-boot-starter:${versions.springfoxSwagger}") api("io.springfox:springfox-swagger2:${versions.springfoxSwagger}") api("io.swagger:swagger-annotations:${versions.swagger}") api("javax.annotation:javax.annotation-api:1.3.2") api("javax.xml.bind:jaxb-api:2.3.1") - api("mysql:mysql-connector-java:8.0.20") api("net.logstash.logback:logstash-logback-encoder:4.11") api("org.apache.commons:commons-exec:1.3") - // TODO(jvz): beginning in BC 1.71, this should use the -jdk18on artifacts - api("org.bouncycastle:bcpkix-jdk15on:${versions.bouncycastle}") - api("org.bouncycastle:bcprov-jdk15on:${versions.bouncycastle}") + // from BC 1.71, module names changed from *-jdk15on to *-jdk18on + // due to this change, some of the modules in downstream services like clouddriver, gate would fall back to + // lower versions(<1.70) as transitive dependencies. So to make them use BC >=1.74(CVE free versions), + // the following dependencies are upgraded: oci-java-sdk-bom, google-cloud-secretmanager + // and in some cases *-jdk15on libraries are excluded so as to use the available *-jdk18on or *-jdk15to18 + // some of the modules would still use <1.70 as they can't be upgraded without upgrading spring boot + api("org.bouncycastle:bcpkix-jdk18on:${versions.bouncycastle}") + api("org.bouncycastle:bcprov-jdk18on:${versions.bouncycastle}") api("org.jetbrains:annotations:19.0.0") api("org.spekframework.spek2:spek-dsl-jvm:${versions.spek2}") api("org.spekframework.spek2:spek-runner-junit5:${versions.spek2}") @@ -209,22 +155,9 @@ dependencies { api("org.jetbrains.spek:spek-junit-platform-engine:${versions.spek}") api("org.jetbrains.spek:spek-junit-platform-runner:${versions.spek}") api("org.jetbrains.spek:spek-subject-extension:${versions.spek}") - api("org.jooq:jooq:${versions.jooq}") - api("org.jooq:jooq-kotlin:${versions.jooq}") - - // The liquibase version starting from 4.0.0 till 4.12.0 - // has an issue w.r.t parsing the changelog file, - // if found at multiple places within the classpath - // https://github.com/liquibase/liquibase/issues/2818 - // This duplicate changelog issue is fixed in 4.13.0, - // but identified another issue that gets introduced in 4.13.0. - // This issue(https://github.com/liquibase/liquibase/issues/3091) - // hinders the migration of sql scripts available in orca - // (https://github.com/spinnaker/orca/tree/master/orca-sql/src/main/resources/db/changelog), - // containing `afterColumn` construct, with a validation error for postgresql. So pin with 3.10.3 api("org.liquibase:liquibase-core"){ version { - strictly "3.10.3" + strictly "4.24.0" } } api("org.objenesis:objenesis:2.5.1") @@ -240,24 +173,21 @@ dependencies { // when we upgrade to spring boot 2.6.x. It's safe to upgrade beyond 1.29 // with spring boot >= 2.6.12. See // https://github.com/spring-projects/spring-boot/issues/32228#issue-136185850.0. - api("org.yaml:snakeyaml:1.27") + // making it strict as some of the modules have it resolved to higher versions (ex: kork-secrets-gcp to 1.30) + api("org.yaml:snakeyaml") { + version { + strictly "1.27" + } + } api("org.springdoc:springdoc-openapi-webmvc-core:${versions.openapi}") api("org.springdoc:springdoc-openapi-kotlin:${versions.openapi}") api("org.springframework.boot:spring-boot-configuration-processor:${versions.springBoot}") api("org.springframework.security.oauth.boot:spring-security-oauth2-autoconfigure:2.3.12.RELEASE") api("org.springframework.security.extensions:spring-security-saml-dsl-core:1.0.5.RELEASE") api("org.springframework.security.extensions:spring-security-saml2-core:1.0.9.RELEASE") - api("org.testng:testng:7.4.0") // TODO: remove this with upgrade of spring-boot version to 2.5.0 or with upgrade of groovy-all to 3.0.8 api("org.threeten:threeten-extra:1.0") api("org.apache.tomcat.embed:tomcat-embed-core:${versions.tomcat}") api("org.apache.tomcat.embed:tomcat-embed-el:${versions.tomcat}") api("org.apache.tomcat.embed:tomcat-embed-websocket:${versions.tomcat}") - api('org.apache.ant:ant:1.10.12') { - because "1.9.15 is resolved transitively through Groovy, removes CVE-2021-36374, CVE-2021-36373, CVE-2020-11979, CVE-2020-15250" - } - api('org.apache.ant:ant-launcher:1.10.12'){ - because "1.9.15 is resolved transitively through Groovy, removes CVE-2021-36374, CVE-2021-36373, CVE-2020-11979, CVE-2020-15250" - } - } }