You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "saie-ch (via GitHub)" <gi...@apache.org> on 2026/03/09 19:38:46 UTC
[PR] feat(go): add TLS integration tests and examples [iggy]
saie-ch opened a new pull request, #2898:
URL: https://github.com/apache/iggy/pull/2898
## Which issue does this PR close?
Closes #2807
## Rationale
The Go SDK lacked TLS integration tests to verify encrypted TCP connections work correctly. Python and other SDKs have similar tests using
testcontainers for portable, self-contained testing.
## What changed?
**Before:** Go SDK had no tests for TLS connections.
**After:** Added 3 integration tests using testcontainers that verify:
1. TLS connections with CA certificates succeed
2. Non-TLS connections to TLS servers are rejected
3. Complete message flows (create stream/topic, send/poll messages) work over TLS
Tests use `apache/iggy:edge` Docker image with testcontainers-go v0.32.0. Also updated getting-started examples with TLS flags and enhanced the test script to run both non-TLS and TLS passes.
**Breaking change:** Bumped Go requirement from 1.23.0 to 1.25.0 (required by testcontainers-go dependency). Go 1.25.0 was released 6 months ago and is stable.
## Local Execution
All TLS integration tests passed locally:
Ran complete pre-merge check suite (format, tidy, lint, tests) - all passed.
## AI Usage
Claude Code (claude-sonnet-4.5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go): add TLS integration tests and examples [iggy]
Posted by "atharvalade (via GitHub)" <gi...@apache.org>.
atharvalade commented on code in PR #2898:
URL: https://github.com/apache/iggy/pull/2898#discussion_r2916079676
##########
foreign/go/client/tcp/tcp_core.go:
##########
Review Comment:
If `createTLSConfig() `fails here, the TCP connection opened by `net.Dial` a few lines above just gets abandoned. Nothing closes it. The handshake failure path right below already does `connection.Close() `but this early return was missed.
This gets worse because the retry loop defaults to infinite retries. So if the TLS config keeps failing (bad CA path, for example), every retry opens a new TCP socket and leaks it. You will eventually run out of file descriptors.
Add `_ = connection.Close()` before the return to match what the handshake error path already does.
##########
scripts/run-go-examples-from-readme.sh:
##########
@@ -169,9 +169,85 @@ cd ../..
kill -TERM "$(cat ${PID_FILE})"
test -e ${PID_FILE} && rm ${PID_FILE}
+# --- TLS Test Pass ---
+if [ "${exit_code}" -eq 0 ]; then
+ echo ""
+ echo -e "\e[36m=== Starting TLS test pass ===\e[0m"
+ echo ""
+
+ # Clean data and logs for fresh TLS start
+ rm -fr local_data
+ rm -f ${LOG_FILE}
+
+ # Start server with TLS enabled
+ echo "Starting server with TLS enabled..."
+ IGGY_ROOT_USERNAME=iggy IGGY_ROOT_PASSWORD=iggy IGGY_TCP_TLS_ENABLED=true ${SERVER_BIN} &>${LOG_FILE} &
+ echo $! >${PID_FILE}
+
+ # Wait for server to start
+ SERVER_START_TIME=0
+ while ! grep -q "has started" ${LOG_FILE}; do
+ if [ ${SERVER_START_TIME} -gt ${TIMEOUT} ]; then
+ echo "TLS server did not start within ${TIMEOUT} seconds."
+ ps fx
+ cat ${LOG_FILE}
+ exit 1
+ fi
+ echo "Waiting for Iggy TLS server to start... ${SERVER_START_TIME}"
+ sleep 1
+ ((SERVER_START_TIME += 1))
+ done
+
+ # Verify TLS is enabled
+ if ! grep -q "tls: { enabled: true" ${LOG_FILE}; then
+ echo -e "\e[31mError: TLS not enabled on server\e[0m"
+ grep -A 5 "tcp:" ${LOG_FILE} || true
+ exit 1
+ fi
+ echo -e "\e[32m✓ TLS enabled on server\e[0m"
+
+ cd examples/go || exit 1
+
+ # Run getting-started examples with TLS flags
+ # Use localhost instead of 127.0.0.1 to match the cert's SAN (DNS:localhost)
+ TLS_CA_FILE="../../core/certs/iggy_ca_cert.pem"
+ TLS_ADDR="localhost:8090"
+
+ for cmd in \
+ "go run getting-started/producer/main.go --tcp-server-address ${TLS_ADDR} --tls --tls-ca-file ${TLS_CA_FILE}" \
+ "go run getting-started/consumer/main.go --tcp-server-address ${TLS_ADDR} --tls --tls-ca-file ${TLS_CA_FILE}"; do
+
+ echo -e "\e[33mChecking TLS example:\e[0m ${cmd}"
+ echo ""
+
+ set +e
+ eval "timeout 10 ${cmd}"
+ test_exit_code=$?
+ set -e
+
+ if [[ $test_exit_code -ne 0 && $test_exit_code -ne 124 ]]; then
+ echo ""
+ echo -e "\e[31mTLS example command failed:\e[0m ${cmd}"
+ echo ""
+ exit_code=$test_exit_code
+ break
+ fi
+ sleep 2
+ done
+
+ cd ../..
+
+ # Terminate TLS server
+ kill -TERM "$(cat ${PID_FILE})"
+ kill -TERM "$(cat ${PID_FILE})" 2>/dev/null || true
Review Comment:
There are two back-to-back kill -TERM calls on the same PID. The first one has no error handling, and the second one has `2>/dev/null || true`. Since the script uses set -e, if the process already exited before line 241 runs, the first kill will fail and the script will abort before it even reaches the safe version on line 242.
##########
foreign/go/tests/tls_test.go:
##########
@@ -0,0 +1,296 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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 tests_test contains TLS integration tests for the Go SDK.
+//
+// These tests use testcontainers to spin up a TLS-enabled Iggy server,
+// making them fully self-contained and suitable for CI.
+//
+// The IGGY_SERVER_DOCKER_IMAGE environment variable must be set to a locally
+// built Docker image. Do NOT use images from DockerHub — CI must test the code
+// from the current PR, not a previously released version.
+//
+// Build the image locally:
+//
+// cargo build && docker build -f core/server/Dockerfile \
+// --build-arg PREBUILT_IGGY_SERVER=target/debug/iggy-server \
+// --build-arg PREBUILT_IGGY_CLI=target/debug/iggy \
+// --build-arg LIBC=glibc --target runtime-prebuilt -t iggy-server:test .
+//
+// Then run tests:
+//
+// IGGY_SERVER_DOCKER_IMAGE=iggy-server:test go test -v ./tests
+//
+// In CI, the image is built by .github/actions/utils/docker-build-test-server/action.yml
+package tests_test
+
+import (
+ "context"
+ "fmt"
+ "net"
+ "os"
+ "path/filepath"
+ "testing"
+ "time"
+
+ "github.com/apache/iggy/foreign/go/client"
+ "github.com/apache/iggy/foreign/go/client/tcp"
+ iggcon "github.com/apache/iggy/foreign/go/contracts"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ "github.com/testcontainers/testcontainers-go"
+ "github.com/testcontainers/testcontainers-go/wait"
+)
+
+const (
+ defaultUsername = "iggy"
+ defaultPassword = "iggy"
+)
+
+// setupTLSContainer starts a TLS-enabled Iggy server container.
+func setupTLSContainer(t *testing.T) (testcontainers.Container, string, string) {
+ ctx := context.Background()
+
+ repoRoot, err := filepath.Abs(filepath.Join("..", "..", ".."))
+ require.NoError(t, err)
+ certsPath := filepath.Join(repoRoot, "core", "certs")
+ caFile := filepath.Join(certsPath, "iggy_ca_cert.pem")
+
+ // Verify certs exist
+ _, err = os.Stat(certsPath)
+ require.NoError(t, err, "Certs directory not found at %s", certsPath)
+ _, err = os.Stat(caFile)
+ require.NoError(t, err, "CA cert not found at %s", caFile)
+
+ // Require locally built Docker image — never pull from DockerHub.
+ // In CI, the image is built by docker-build-test-server action.
+ // Locally, build with: cargo build && docker build -f core/server/Dockerfile ...
+ dockerImage := os.Getenv("IGGY_SERVER_DOCKER_IMAGE")
+ if dockerImage == "" {
+ t.Fatal("IGGY_SERVER_DOCKER_IMAGE env var is required. " +
Review Comment:
Both the C# and Python SDKs fall back to `apache/iggy:edge `when `IGGY_SERVER_DOCKER_IMAGE` is not set. This hard-fails instead, which makes running these tests locally more cumbersome than in the other SDKs. Consider defaulting to `apache/iggy:edge` to match the established pattern across the project.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go): add TLS integration tests and examples [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio merged PR #2898:
URL: https://github.com/apache/iggy/pull/2898
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go): add TLS integration tests and examples [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #2898:
URL: https://github.com/apache/iggy/pull/2898#issuecomment-4026919684
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/2898?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 68.45%. Comparing base ([`3bcf7ff`](https://app.codecov.io/gh/apache/iggy/commit/3bcf7ff5e23d693c8033f2a672901ea412298c52?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`737bca8`](https://app.codecov.io/gh/apache/iggy/commit/737bca850acb7349339bc4e966ebb5d02fc22e04?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 5 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #2898 +/- ##
============================================
- Coverage 70.32% 68.45% -1.88%
Complexity 739 739
============================================
Files 1037 962 -75
Lines 85614 79756 -5858
Branches 62144 61970 -174
============================================
- Hits 60210 54598 -5612
+ Misses 22903 22654 -249
- Partials 2501 2504 +3
```
| [Flag](https://app.codecov.io/gh/apache/iggy/pull/2898/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [node](https://app.codecov.io/gh/apache/iggy/pull/2898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.29% <ø> (-5.12%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
[see 93 files with indirect coverage changes](https://app.codecov.io/gh/apache/iggy/pull/2898/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org