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

zarf tries to apply template replacements to large files that might be incorrectly assessed as text #2308

Closed
WeaponX314 opened this issue Feb 16, 2024 · 1 comment · Fixed by #2310
Labels
bug 🐞 Something isn't working

Comments

@WeaponX314
Copy link
Contributor

WeaponX314 commented Feb 16, 2024

Environment

Device and OS: linux/amd64
App version: 0.32.3
Kubernetes distro being used: k3s (Not applicable for this problem)
Other:

Steps to reproduce

  1. Create a zarf package that pulls a self-executing script like the NVIDIA script:
    files:
      - source: https://us.download.nvidia.com/XFree86/Linux-x86_64/535.154.05/NVIDIA-Linux-x86_64-535.154.05.run
        target: /tmp/nvidia-driver.run
        executable: true
  1. Attempt to deploy it using zarf package deploy

Expected result

FIle is simply copied to location

Actual Result

This particular type of file is technically a script with an archive concatenated with it and it is rather large. Zarf detects it as a text file since it has text up front and tries to apply templating (###ZARF_VAR_*) to it.

There should be a way perhaps in the zarf.yaml to force it to not take this behavior. Maybe something like this?

    files:
      - source: https://us.download.nvidia.com/XFree86/Linux-x86_64/535.154.05/NVIDIA-Linux-x86_64-535.154.05.run
        target: /tmp/nvidia-driver.run
        executable: true
        noTemplate: true                  // new optional boolean

OR

Change the detection to also check the last couple of bytes at the end of the file too, should not be text for these type of installers.

If you opt for the latter, I can probably submit a PR myself. If you want to do the former, I would prefer your team discuss how you want to proceed. Happy to help where I can!

Visual Proof (screenshots, videos, text, etc)

  📦 DRIVERS COMPONENT                                                                                
                                                                                                      

  ⠦  Templating /tmp/nvidia-driver.run (9m10s)  

Severity/Priority

I can work around this by using an action to tar.gz it up beforehand but would be nice if I didn't have to do that.

Additional Context

Looks like this is the area of concern:

https://github.com/defenseunicorns/zarf/blob/7e91d3b9823b52fe6d0f563d692c8af57faa6005/src/pkg/packager/deploy.go#L360

@Racer159 Racer159 added bug 🐞 Something isn't working and removed possible-bug 🐛 labels Feb 16, 2024
@Racer159
Copy link
Contributor

IMO we would want to add more detection logic to root out these kinds of files (rather than add a new key) - this could be done in two ways:

  1. As you mentioned, check the last 512 bytes for binary data bytes (we wouldn't be able to fully trust the http.DetectContentType mimetype since it would be missing things like byte order marks but it likely would give a decent clue)
    https://mimesniff.spec.whatwg.org/#sniffing-a-mislabeled-binary-resource
  2. Simply check the file size and return if it exceeds ~20MiB - k8s resources are usually limited to ~1MiB in size and while these can be stitched together, once you reach these higher sizes the likelihood these are resources to template goes down significantly (and it is something to discourage anyway because it would be a lot to process). If we go this route we should add a warning when we are refusing to template a file though.

1 is probably best to explore first since theoretically it is more flexible if it can properly detect these files.

Racer159 added a commit that referenced this issue Feb 21, 2024
## Description

Alters text detection logic to read the first and last 512 bytes.

Tested with 5 files:
- [NVIDIA
installer](https://us.download.nvidia.com/XFree86/Linux-x86_64/535.154.05/NVIDIA-Linux-x86_64-535.154.05.run)
    Detected as application type when reading last 512.
- 3 4k size files of junk text with a ZARF_CONST replacement, in
straight text, yaml, and json
    All 3 detected as text/plain, ZARF_CONST was replaced.
- 1 small 100 byte file with a ZARF_CONST replacement.
   Was still detected as text and ZARF_CONST was replaced.
  
Existing unit tests succeeded.

## Related Issue

Fixes #2308
<!-- or -->
Relates to #

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow)
followed

---------

Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants