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.

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

Jump to bottom

tctl sign auth --tar is failing to run with kubernetes output format #34371

Closed
salehio opened this issue Nov 9, 2023 · 0 comments · Fixed by #34612
Closed

tctl sign auth --tar is failing to run with kubernetes output format #34371

salehio opened this issue Nov 9, 2023 · 0 comments · Fixed by #34612
Assignees
@tcsc
Labels
bug

Comments

@salehio
Copy link

salehio commented Nov 9, 2023

Expected behavior:
tctl auth sign -d --user={user} --ttl={identity_ttl} --tar --out={outfile} --format=kubernetes --proxy={proxy} --kube-cluster-name={cluster}

This should output the kubeconfig (tar'd) to stdout

Current behavior:
The command results in:
Results in


2023-11-09T00:18:06Z DEBU             Debug logging has been enabled. common/tctl.go:240
2023-11-09T00:18:06Z DEBU             No config file or identity file, loading auth config via extension. common/tctl.go:286
2023-11-09T00:18:06Z DEBU [SQLITE]    Connected to: file:/var/lib/teleport/proc/sqlite.db?_busy_timeout=10000&_sync=FULL&_txlock=immediate, poll stream period: 1s lite/lite.go:250
2023-11-09T00:18:06Z DEBU [SQLITE]    journal_mode=delete, synchronous=2, busy_timeout=10000 lite/lite.go:301
2023-11-09T00:18:06Z DEBU             Connecting to: [{127.0.0.1:3025 tcp }]. authclient/authclient.go:57
2023-11-09T00:18:06Z DEBU             Using Proxy SNI for kube TLS server name common/auth_command.go:877

ERROR REPORT:
Original Error: *trace.NotImplementedError tarWriter.Remove()
Stack Trace:
        github.com/gravitational/teleport/tool/tctl/common/tarwriter.go:49 github.com/gravitational/teleport/tool/tctl/common.(*tarWriter).Remove
        github.com/gravitational/teleport/lib/client/identityfile/identity.go:401 github.com/gravitational/teleport/lib/client/identityfile.Write
        github.com/gravitational/teleport/tool/tctl/common/auth_command.go:899 github.com/gravitational/teleport/tool/tctl/common.(*AuthCommand).generateUserKeys
        github.com/gravitational/teleport/tool/tctl/common/auth_command.go:289 github.com/gravitational/teleport/tool/tctl/common.(*AuthCommand).GenerateAndSignKeys
        github.com/gravitational/teleport/tool/tctl/common/auth_command.go:172 github.com/gravitational/teleport/tool/tctl/common.(*AuthCommand).TryRun
        github.com/gravitational/teleport/tool/tctl/common/tctl.go:211 github.com/gravitational/teleport/tool/tctl/common.TryRun
        github.com/gravitational/teleport/tool/tctl/common/tctl.go:96 github.com/gravitational/teleport/tool/tctl/common.Run
        github.com/gravitational/teleport/e/tool/tctl/main.go:20 main.main
        runtime/proc.go:250 runtime.main
        runtime/asm_amd64.s:1598 runtime.goexit
User Message: tarWriter.Remove()

command [REDACTED] failed with code 1

Note the Original Error: *trace.NotImplementedError tarWriter.Remove()

Here we can see that tarWriter.Remove() is not implemented.
https://github.com/gravitational/teleport/blob/master/tool/tctl/common/tarwriter.go#L46-L50

But it is still invoked when the format is kubernetes: https://github.com/gravitational/teleport/blob/master/lib/client/identityfile/identity.go#L401-L403

Since the images have gone distroless, we can no longer kubectl cp or kubectl exec -- cat the auth file out of the box, so we need to rely on that tar implementation. See #27639 for more details on why --tar was implemented.
@salehio salehio added the bug label Nov 9, 2023
tcsc added a commit that referenced this issue Nov 15, 2023
@tcsc
Fixes crash when writing kubeconfig with tctl auth sign --tar
d709c7a 
Prior this patch, using the `--format=kubernetes` option with
`tctl auth sign --tar` would crash due to the filesystem abstraction
used to capture the `tctl` output files did not support removing or
`stat`ing files.

In addition, the kubeconfig file writer did not use the filesystem
abstraction given to the identity file writer, but would only write
files out to the host filesystem. This means that any kubeconfig
file oututs would not be included in the output tarfile stream.

This patch:
 * Updates the tarfile filesystem abstraction to buffer files created
   by `tctl` until the write is complete, and then stream the archive
   out at the end. This gives sensible semabtics to the remove and
   stat operations.
 * Updates the kubeconfig writer to take a filesystem abstraction
   compatible with the one used by the identity file writer, so that
   the kubeconfg file output is correctly caught by the tarfile writer.

Fixes: #34371
Changelog: Fixes crash when writing kubeconfig with `tctl auth sign --tar`
@tcsc tcsc mentioned this issue Nov 15, 2023
Merged
github-merge-queue bot pushed a commit that referenced this issue Nov 21, 2023
@tcsc
Fixes crash when writing kubeconfig with tctl auth sign --tar (#34612)
34a5ab7 
* Fixes crash when writing kubeconfig with `tctl auth sign --tar`

Prior this patch, using the `--format=kubernetes` option with
`tctl auth sign --tar` would crash due to the filesystem abstraction
used to capture the `tctl` output files did not support removing or
`stat`ing files.

In addition, the kubeconfig file writer did not use the filesystem
abstraction given to the identity file writer, but would only write
files out to the host filesystem. This means that any kubeconfig
file oututs would not be included in the output tarfile stream.

This patch:
 * Updates the tarfile filesystem abstraction to buffer files created
   by `tctl` until the write is complete, and then stream the archive
   out at the end. This gives sensible semabtics to the remove and
   stat operations.
 * Updates the kubeconfig writer to take a filesystem abstraction
   compatible with the one used by the identity file writer, so that
   the kubeconfg file output is correctly caught by the tarfile writer.

Fixes: #34371
Changelog: Fixes crash when writing kubeconfig with `tctl auth sign --tar`

* iterface & lint fixups

* Fix kubecofig loader origin settings

* Re-route tar-breaking log msg to stderr
@tcsc tcsc mentioned this issue Nov 21, 2023
Merged
github-merge-queue bot pushed a commit that referenced this issue Nov 21, 2023
@tcsc
[v13] Fixes crash when writing kubeconfig with tctl auth sign --tar (…
121b3d9 
…#34822)

* [v13] Fixes crash when writing kubeconfig with `tctl auth sign --tar`

Backports #34612

Prior this patch, using the `--format=kubernetes` option with
`tctl auth sign --tar` would crash due to the filesystem abstraction
used to capture the `tctl` output files did not support removing or
`stat`ing files.

In addition, the kubeconfig file writer did not use the filesystem
abstraction given to the identity file writer, but would only write
files out to the host filesystem. This means that any kubeconfig
file oututs would not be included in the output tarfile stream.

This patch:
 * Updates the tarfile filesystem abstraction to buffer files created
   by `tctl` until the write is complete, and then stream the archive
   out at the end. This gives sensible semabtics to the remove and
   stat operations.
 * Updates the kubeconfig writer to take a filesystem abstraction
   compatible with the one used by the identity file writer, so that
   the kubeconfg file output is correctly caught by the tarfile writer.

Fixes: #34371
Changelog: Fixes crash when writing kubeconfig with `tctl auth sign --tar`

* Fix backport test detritus
@tcsc tcsc mentioned this issue Nov 22, 2023
Merged
github-merge-queue bot pushed a commit that referenced this issue Dec 5, 2023
@tcsc
[v14] Fixes crash when writing kubeconfig with tctl auth sign --tar (…
8e9cbea 
…#34874)

Backports #34612

Prior this patch, using the `--format=kubernetes` option with
`tctl auth sign --tar` would crash due to the filesystem abstraction
used to capture the `tctl` output files did not support removing or
`stat`ing files.

In addition, the kubeconfig file writer did not use the filesystem
abstraction given to the identity file writer, but would only write
files out to the host filesystem. This means that any kubeconfig
file oututs would not be included in the output tarfile stream.

This patch:
 * Updates the tarfile filesystem abstraction to buffer files created
   by `tctl` until the write is complete, and then stream the archive
   out at the end. This gives sensible semabtics to the remove and
   stat operations.
 * Updates the kubeconfig writer to take a filesystem abstraction
   compatible with the one used by the identity file writer, so that
   the kubeconfg file output is correctly caught by the tarfile writer.

Fixes: #34371
Changelog: Fixes crash when writing kubeconfig with `tctl auth sign --tar`

* Fix backport test detritus
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tcsc tcsc

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes crash when writing kubeconfig with tctl auth sign --tar gravitational/teleport
2 participants
@tcsc @salehio