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

fix: support detecting if k3s module is running from inside a Docker container #1289

Merged
merged 1 commit into from
Jun 15, 2023

Conversation

rfratto
Copy link
Contributor

@rfratto rfratto commented Jun 15, 2023

The k3s module previously didn't work properly inside of a Docker container; the caller would connect to its HTTPS server via the Docker network gateway, which wasn't included in the list of SANS from the TLS certificate exposed by the k3s container. This would cause requests to fail due to TLS validation errors:

Get "https://172.17.0.1:32776/api/v1": tls: failed to verify certificate: x509: certificate is valid for 10.43.0.1, 127.0.0.1, 172.17.0.6, ::1, not 172.17.0.1

This change determines what the DaemonHost address is prior to creating the container request, and then injects that address as one of the TLS SANs.

When not running in a Docker container, the k3s module continues to function as it did before, as the DaemonHost address in that scenario is "localhost".

I have successfully tested this PR both locally and verifying that it fixes the tests in our environment (see grafana/agent#4161).

The k3s module previously didn't work properly inside of a Docker
container; the caller would connect to its HTTPS server via the Docker
network gateway, which wasn't included in the list of SANS from the TLS
certificate exposed by the k3s container.

This change determines what the DaemonHost address is prior to creating
the container request, and then injects that address as one of the TLS
SANs.

When not running in a Docker container, the k3s module continues to
function as it did before, as the DaemonHost address in that scenario is
"localhost".
@rfratto rfratto requested a review from a team as a code owner June 15, 2023 00:23
@netlify
Copy link

netlify bot commented Jun 15, 2023

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 4e2d74f
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/648a59f019450a0008b9b468
😎 Deploy Preview https://deploy-preview-1289--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sonarcloud
Copy link

sonarcloud bot commented Jun 15, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution! It makes sense to distinguish the execution context: from inside a container or not.

Apart from the PR, I'm interested in knowing more about your use case for this module: could you share with us a little bit of how you use it and which use cases it does resolve in your projects? 🙏

if logging == nil {
logging = testcontainers.Logger
}
p, err := req.ProviderType.GetProvider(testcontainers.WithLogger(logging))
Copy link
Member

Choose a reason for hiding this comment

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

I'm taking a look to check if it's possible to get a Provider instance from the library, instead of from the container request + customisers. In fact in https://github.com/testcontainers/testcontainers-go/pull/1251/files#diff-e9b78e9b34e14f076ba250ebd1042b315e63673569f82372a8064cece5f034bfR25 I'm deprecating the field in order to get them automatically from the environment.

In any case, I think this is good enough for now :)

@mdelapenya
Copy link
Member

@mritunjaysharma394, as original contributor of the module, could you also take a look? 🙏

@mdelapenya mdelapenya self-assigned this Jun 15, 2023
@mdelapenya mdelapenya added the bug An issue with the library label Jun 15, 2023
@rfratto
Copy link
Contributor Author

rfratto commented Jun 15, 2023

Apart from the PR, I'm interested in knowing more about your use case for this module: could you share with us a little bit of how you use it and which use cases it does resolve in your projects? 🙏

Sure! We have a Kubernetes operator where we run a Kubernetes cluster to do integration tests. We previously embedded k3d as a library for bringing up a Kubernetes, but started to look for another solution when we realized importing k3d was causing some dependency hell issue. (We have a ton of dependencies already).

We were already using testcontainers-go for bringing up a Vault server for integration tests, so switching out our k3d code for the k3s module made a ton of sense for us.

We'll be making more heavy use of the k3s module over time, as we're starting to add more Kubernetes controllers into our non-operator binary, where integration tests will help with having confidence in the stability of our code.

@mdelapenya mdelapenya changed the title fix: fix k3s module when running from inside a Docker container fix: support detecting if k3s module is running from inside a Docker container Jun 15, 2023
@mdelapenya mdelapenya merged commit c175df3 into testcontainers:main Jun 15, 2023
@rfratto rfratto deleted the fix-k3s-module-in-container branch June 15, 2023 14:27
@mdelapenya
Copy link
Member

@rfratto we plan a release in the short term, but we need to resolve one issue with Windows first (see #1249)

At the moment, each module follows the core library release cycle, as we don't want to fall in the trap of maintaining multiple release branches (one per module), so the next full release will include these changes. 🙏

@rfratto
Copy link
Contributor Author

rfratto commented Jun 15, 2023

No worries! We'll use the main branch until is a release is available. Thanks for the update!

@mdelapenya
Copy link
Member

@rfratto just sharing that v0.21.0 is out, including the k3s module 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue with the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants