Skip to content

Double check that k8s cert file was copied #11461

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.

Sign up for GitHub

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

Open
fasaxc wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Double check that k8s cert file was copied #11461

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
35daba9
Add additional checks/diagnostics to k8s cert copy.
fasaxc Nov 28, 2025
2cbf821
Use --mount for better diags/error on failure.
fasaxc Nov 28, 2025
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
45 changes: 37 additions & 8 deletions felix/fv/infrastructure/infra_k8s.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,20 +455,49 @@ func setupK8sDatastoreInfra(opts ...CreateOption) (kds *K8sDatastoreInfra, err e
kds.CertFileName = "/tmp/" + kds.k8sApiContainer.Name + ".crt"
start = time.Now()
for {
// Make sure any retry is clean.
_ = os.Remove(kds.CertFileName)

cmd := utils.Command("docker", "cp",
kds.k8sApiContainer.Name+":/home/user/certs/kubernetes.pem",
kds.CertFileName,
)
err = cmd.Run()
if err == nil {
break
if err != nil {
if time.Since(start) > 120*time.Second {
log.WithError(err).Error("Failed to get API server cert")
return nil, err
}
time.Sleep(100 * time.Millisecond)
continue
}
if time.Since(start) > 120*time.Second {
log.WithError(err).Error("Failed to get API server cert")
return nil, err
// Sometimes the very first felix container that we start fails to
// mount this file. Double check that it is there and we can read it.
if _, err := os.Stat(kds.CertFileName); err != nil {
log.WithError(err).Error("Failed to stat API server cert that we just copied?!")
if time.Since(start) > 120*time.Second {
return nil, err
}
time.Sleep(100 * time.Millisecond)
continue
}
time.Sleep(100 * time.Millisecond)
if f, err := os.Open(kds.CertFileName); err != nil {
log.WithError(err).Error("Failed to open API server cert that we just copied?!")
if time.Since(start) > 120*time.Second {
return nil, err
}
time.Sleep(100 * time.Millisecond)
continue
} else {
err := f.Sync()
if err != nil {
log.WithError(err).Error("Failed to sync API server cert that we just copied?!")
}
_ = f.Close()
}
break
}
log.Info("Got API server cert.")

start = time.Now()
for {
Expand Down Expand Up @@ -638,7 +667,7 @@ func (kds *K8sDatastoreInfra) GetDockerArgs() []string {
"-e", "K8S_API_ENDPOINT=" + kds.Endpoint,
"-e", "KUBERNETES_MASTER=" + kds.Endpoint,
"-e", "K8S_INSECURE_SKIP_TLS_VERIFY=true",
"-v", kds.CertFileName + ":/tmp/apiserver.crt",
"--mount", fmt.Sprintf("type=bind,source=%s,target=%s", kds.CertFileName, "/tmp/apiserver.crt"),
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

[nitpick] The --mount syntax is missing the readonly option. Since this is a certificate file that should not be modified by the container, consider adding readonly for better security:

"--mount", fmt.Sprintf("type=bind,source=%s,target=%s,readonly", kds.CertFileName, "/tmp/apiserver.crt"),

This prevents accidental modification of the certificate file from within the container.

Copilot uses AI. Check for mistakes.
}
}

Expand All @@ -649,7 +678,7 @@ func (kds *K8sDatastoreInfra) GetBadEndpointDockerArgs() []string {
"-e", "TYPHA_DATASTORETYPE=kubernetes",
"-e", "K8S_API_ENDPOINT=" + kds.BadEndpoint,
"-e", "K8S_INSECURE_SKIP_TLS_VERIFY=true",
"-v", kds.CertFileName + ":/tmp/apiserver.crt",
"--mount", fmt.Sprintf("type=bind,source=%s,target=%s", kds.CertFileName, "/tmp/apiserver.crt"),
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

[nitpick] The --mount syntax is missing the readonly option. Since this is a certificate file that should not be modified by the container, consider adding readonly for better security:

"--mount", fmt.Sprintf("type=bind,source=%s,target=%s,readonly", kds.CertFileName, "/tmp/apiserver.crt"),

This prevents accidental modification of the certificate file from within the container.

Copilot uses AI. Check for mistakes.
}
}

Expand Down