Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kn: checksum bindings #1182

Open
wants to merge 23 commits into
base: kn-main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion .github/actions/setup-build/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,20 @@ description: >
runs:
using: composite
steps:
- name: Checkout tools
- name: Extract aws-kotlin-repo-tools version
working-directory: ./smithy-kotlin
shell: bash
run: |
export AWS_KOTLIN_REPO_TOOLS_VERSION=$(grep '^aws-kotlin-repo-tools-version' ./gradle/libs.versions.toml | sed -E 's/.*= "(.*)"/\1/')
echo "Using aws-kotlin-repo-tools version $AWS_KOTLIN_REPO_TOOLS_VERSION"
echo "aws_kotlin_repo_tools_version=$AWS_KOTLIN_REPO_TOOLS_VERSION" >> $GITHUB_ENV

- name: Checkout aws-kotlin-repo-tools
uses: actions/checkout@v4
with:
path: 'aws-kotlin-repo-tools'
repository: 'awslabs/aws-kotlin-repo-tools'
ref: ${{ env.aws_kotlin_repo_tools_version }}
sparse-checkout: |
.github

Expand All @@ -19,6 +28,7 @@ runs:
# checkout aws-crt-kotlin as a sibling which will automatically make it an included build
path: 'aws-crt-kotlin'
repository: 'awslabs/aws-crt-kotlin'
submodules: 'true'

# Cache the Kotlin/Native toolchain based on the input Kotlin version from version catalog
# see https://kotlinlang.org/docs/native-improving-compilation-time.html
Expand Down
152 changes: 152 additions & 0 deletions .github/scripts/run-container-test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
#!/usr/bin/env python3
#
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
# SPDX-License-Identifier: Apache-2.0

"""
Run precompiled Kotlin/Native test binaries in a Docker container for a specific Linux distribution and architecture.

This requires Docker multiarch support, see https://docs.docker.com/build/building/multi-platform/ and https://github.com/multiarch/qemu-user-static
In GitHub we use a provided action for this: https://github.com/docker/setup-qemu-action

Locally you would need to run one of:

`docker run --rm --privileged multiarch/qemu-user-static --reset -p yes --credential yes`

OR

`docker run --privileged --rm tonistiigi/binfmt --install all`
"""

import argparse
import os
import subprocess
import shlex
import shutil

VERBOSE = False

DISTRO_TO_IMAGE_NAME = {
"ubuntu-22.04": "public.ecr.aws/lts/ubuntu:22.04_stable",
"al2023": "public.ecr.aws/amazonlinux/amazonlinux:2023",
"al2": "public.ecr.aws/amazonlinux/amazonlinux:2"
}

DOCKER_PLATFORM_BY_ARCH = {
"x64": "linux/amd64",
"arm64": "linux/arm64"
}


def vprint(message):
global VERBOSE
if VERBOSE:
print(message)


def running_in_github_action():
"""
Test if currently running in a GitHub action or running locally
:return: True if running in GH, False otherwise
"""
return "GITHUB_WORKFLOW" in os.environ


def shell(command, cwd=None, check=True, capture_output=False):
"""
Run a command
:param command: command to run
:param cwd: the current working directory to change to before executing the command
:param check: flag indicating if the status code should be checked. When true an exception will be
thrown if the command exits with a non-zero exit status.
:returns: the subprocess CompletedProcess output
"""
vprint(f"running `{command}`")
return subprocess.run(command, shell=True, check=check, cwd=cwd, capture_output=capture_output)


def oci_executable():
"""
Attempt to find the OCI container executor used to build and run docker containers
"""
oci_exe = os.environ.get('OCI_EXE')
if oci_exe is not None:
return oci_exe

executors = ['finch', 'podman', 'docker']

for exe in executors:
if shutil.which(exe) is not None:
return exe

print("cannot find container executor")
exit(1)


def run_docker_test(opts):
"""
Run a docker test for a precompiled Kotlin/Native binary

:param opts: the parsed command line options
"""
platform = DOCKER_PLATFORM_BY_ARCH[opts.arch]
oci_exe = oci_executable()

test_bin_dir = os.path.abspath(opts.test_bin_dir)
image_name = DISTRO_TO_IMAGE_NAME[opts.distro]
path_to_exe = f'./linux{opts.arch.capitalize()}/debugTest/test.kexe'

cmd = [
oci_exe,
'run',
'--rm',
f'-v{test_bin_dir}:/test',
]
if not opts.no_system_certs:
cmd.append(f'-v/etc/ssl:/etc/ssl')
Comment on lines +105 to +106
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: f is unnecessary since we're not formatting any parameters in this string


cmd.extend(
[
'-w/test',
'-e DEBIAN_FRONTEND=noninteractive',
'--platform',
platform,
image_name,
path_to_exe,
]
)

cmd = shlex.join(cmd)
print(cmd)
shell(cmd)


def create_cli():
parser = argparse.ArgumentParser(
prog="run-container-test",
description="Run cross platform test binaries in a container",
formatter_class=argparse.ArgumentDefaultsHelpFormatter
)

parser.add_argument("-v", "--verbose", help="enable verbose output", action="store_true")

parser.add_argument("--distro", required=True, choices=DISTRO_TO_IMAGE_NAME.keys(), help="the distribution name to run the task on")
parser.add_argument("--arch", required=True, choices=DOCKER_PLATFORM_BY_ARCH.keys(), help="the architecture to use")
parser.add_argument("--test-bin-dir", required=True, help="the path to the test binary directory root")
parser.add_argument("--no-system-certs", action='store_true', help="disable mounting system certificates into the container")

return parser


def main():
cli = create_cli()
opts = cli.parse_args()
if opts.verbose:
global VERBOSE
VERBOSE = True

run_docker_test(opts)


if __name__ == '__main__':
main()
22 changes: 22 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,3 +223,25 @@ If you discover a potential security issue in this project we ask that you notif
See the [LICENSE](LICENSE) file for our project's licensing. We will ask you to confirm the licensing of your contribution.

We may ask you to sign a [Contributor License Agreement (CLA)](http://en.wikipedia.org/wiki/Contributor_License_Agreement) for larger changes.

## Kotlin/Native
Support for Kotlin/Native is in active development. The following sections cover how to effectively test your work.

### Linux/Unix

#### Testing Different Linux Distros and Architectures

1. Build the test executable(s) for the architecture(s) you want to test.

```sh
# build specific arch
./gradlew linuxX64TestBinaries
```

2. Use the `run-container-test.py` helper script to execute tests locally. Change the directory as needed.

```sh
OCI_EXE=docker python3 .github/scripts/run-container-test.py --distro al2 --arch x64 --test-bin-dir ./runtime/runtime-core/build/bin
```

See the usage/help for different distributions provided: `python3 .github/scripts/run-container.py -h`
3 changes: 0 additions & 3 deletions bom/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension
import org.jetbrains.kotlin.gradle.plugin.KotlinMultiplatformPluginWrapper
import org.jetbrains.kotlin.gradle.plugin.KotlinTarget
import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinMetadataTarget
import org.jetbrains.kotlin.gradle.targets.js.KotlinJsTarget
import java.util.*

plugins {
`maven-publish`
Expand Down Expand Up @@ -52,7 +50,6 @@ fun createBomConstraintsAndVersionCatalog() {

fun Project.artifactId(target: KotlinTarget): String = when (target) {
is KotlinMetadataTarget -> name
is KotlinJsTarget -> "$name-js"
else -> "$name-${target.targetName.lowercase()}"
}

Expand Down
3 changes: 2 additions & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
kotlin-version = "2.0.21"
dokka-version = "1.9.10"

aws-kotlin-repo-tools-version = "0.4.15-kn"
aws-kotlin-repo-tools-version = "0.4.18-kn"

# libs
coroutines-version = "1.9.0"
Expand Down Expand Up @@ -96,6 +96,7 @@ ktor-server-netty = { module = "io.ktor:ktor-server-netty", version.ref = "ktor-
ktor-server-jetty-jakarta = { module = "io.ktor:ktor-server-jetty-jakarta", version.ref = "ktor-version" }
ktor-server-cio = { module = "io.ktor:ktor-server-cio", version.ref = "ktor-version" }
ktor-network-tls-certificates = { module = "io.ktor:ktor-network-tls-certificates", version.ref = "ktor-version" }
ktor-client-core = { module = "io.ktor:ktor-client-core", version.ref = "ktor-version" }

kaml = { module = "com.charleskorn.kaml:kaml", version.ref = "kaml-version" }
jsoup = { module = "org.jsoup:jsoup", version.ref = "jsoup-version" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,4 @@ import aws.smithy.kotlin.runtime.auth.awssigning.tests.SigningSuiteTestBase

class CrtSigningSuiteTest : SigningSuiteTestBase() {
override val signer: AwsSigner = CrtAwsSigner

override val disabledTests = super.disabledTests + setOf(
// FIXME - Signature mismatch possibly related to https://github.com/awslabs/aws-crt-java/pull/419. Needs
// investigation.
"get-utf8",
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import aws.smithy.kotlin.runtime.http.request.url
import aws.smithy.kotlin.runtime.httptest.TestWithLocalServer
import aws.smithy.kotlin.runtime.net.Host
import aws.smithy.kotlin.runtime.net.Scheme
import aws.smithy.kotlin.runtime.testing.IgnoreWindows
import io.ktor.server.cio.*
import io.ktor.server.engine.*
import io.ktor.server.response.*
Expand All @@ -25,9 +24,7 @@ import kotlinx.coroutines.yield
import kotlin.test.Test
import kotlin.time.Duration.Companion.seconds

// FIXME This test implements [TestWithLocalServer] which is JVM-only.
class AsyncStressTest : TestWithLocalServer() {

override val server = embeddedServer(CIO, serverPort) {
routing {
get("/largeResponse") {
Expand All @@ -37,9 +34,8 @@ class AsyncStressTest : TestWithLocalServer() {
call.respondText(text.repeat(respSize / text.length))
}
}
}
}.start()

@IgnoreWindows("FIXME - times out after upgrade to kotlinx.coroutines 1.6.0")
@Test
fun testStreamNotConsumed() = runBlocking {
// test that filling the stream window and not consuming the body stream still cleans up resources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class SendChunkedBodyTest {

val chunkedBytes = """
100;chunk-signature=${"0".repeat(64)}\r\n${"0".repeat(256)}\r\n\r\n
""".trimIndent().toByteArray()
""".trimIndent().encodeToByteArray()

val source = chunkedBytes.source()

Expand All @@ -52,7 +52,7 @@ class SendChunkedBodyTest {

val chunkedBytes = """
${chunkSize.toString(16)};chunk-signature=${"0".repeat(64)}\r\n${"0".repeat(chunkSize)}\r\n\r\n
""".trimIndent().toByteArray()
""".trimIndent().encodeToByteArray()

val source = chunkedBytes.source()

Expand All @@ -71,7 +71,7 @@ class SendChunkedBodyTest {

val chunkedBytes = """
100;chunk-signature=${"0".repeat(64)}\r\n${"0".repeat(256)}\r\n\r\n
""".trimIndent().toByteArray()
""".trimIndent().encodeToByteArray()

val channel = SdkByteReadChannel(chunkedBytes)

Expand All @@ -91,7 +91,7 @@ class SendChunkedBodyTest {

val chunkedBytes = """
${chunkSize.toString(16)};chunk-signature=${"0".repeat(64)}\r\n${"0".repeat(chunkSize)}\r\n\r\n
""".trimIndent().toByteArray()
""".trimIndent().encodeToByteArray()

val channel = SdkByteReadChannel(chunkedBytes)

Expand Down
8 changes: 8 additions & 0 deletions runtime/protocol/http-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ kotlin {
}
}

jvmAndNativeMain {
dependencies {
api(libs.ktor.server.cio)
implementation(libs.ktor.client.core)
implementation(libs.ktor.server.core)
}
}

all {
languageSettings.optIn("aws.smithy.kotlin.runtime.InternalApi")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@

package aws.smithy.kotlin.runtime.httptest

import io.ktor.server.engine.*
import io.ktor.server.engine.EmbeddedServer
import kotlinx.coroutines.delay
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeout
import java.net.*
import java.util.concurrent.*
import kotlin.test.AfterTest
import kotlin.test.BeforeTest
import kotlin.time.Duration.Companion.seconds
Expand All @@ -19,7 +18,7 @@ import kotlin.time.Duration.Companion.seconds
* mocking an HTTP client engine is difficult.
*/
public abstract class TestWithLocalServer {
protected val serverPort: Int = ServerSocket(0).use { it.localPort }
protected val serverPort: Int = 54734
Comment on lines -22 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: This will conflict if any other process (including a parallel build of this package in another workspace) is using port 54734. Is there a reason why it's necessary to use a static port?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't configure a port, it will default to 80, we could make it a randomly selected port if you think that will be safer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random ports are marginally safer but still could conflict with an open port. I think we can still bind to an available address using Ktor's low-level KMP socket interfaces:

protected val serverPort: Int = runBlocking {
    SelectorManager(coroutineContext).use { selector ->
        aSocket(selector)
            .tcp()
            .bind()
            .use { (it.localAddress as InetSocketAddress).port }
    }
}

protected val testHost: String = "localhost"

public abstract val server: EmbeddedServer<*, *>
Expand All @@ -33,32 +32,18 @@ public abstract class TestWithLocalServer {
attempt++
try {
server.start()
println("test server listening on: $testHost:$serverPort")
break
} catch (cause: Throwable) {
if (attempt >= 10) throw cause
Thread.sleep(250L * attempt)
delay(250L * attempt)
}
} while (true)

ensureServerRunning()
Comment on lines 32 to -44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Isn't server.start() asynchronous? Won't removing ensureServerRunning() mean that it's possible to return from this method before the server is actually running?

}
}

@AfterTest
public fun stopServer() {
server.stop(0, 0, TimeUnit.SECONDS)
server.stop(0, 0)
println("test server stopped")
}

private fun ensureServerRunning() {
do {
try {
Socket("localhost", serverPort).close()
break
} catch (_: Throwable) {
Thread.sleep(100)
}
} while (true)
}
}
Loading
Loading