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

Package create time relative path issues using imports #1756

Closed
WeaponX314 opened this issue May 26, 2023 · 2 comments · Fixed by #1758
Closed

Package create time relative path issues using imports #1756

WeaponX314 opened this issue May 26, 2023 · 2 comments · Fixed by #1758
Labels
bug 🐞 Something isn't working

Comments

@WeaponX314
Copy link
Contributor

WeaponX314 commented May 26, 2023

Environment

Device and OS: Linux
App version: 0.27.0
Kubernetes distro being used: k3s, but really N/A here
Other: test works with 0.26.1

Steps to reproduce

  1. Look at attached tar.gz for complete working example that completes successfully on 0.26.1 but fails on 0.27.0
  2. Copy this to some arbitrary directory and extract
  3. Run zarf package create

Expected result

  1. Expect a package to be created

Actual Result

Failed to create package: unable to compose component test-component: unable to get child component: unable to fix composed filepaths (see visual proof for more details)

Visual Proof (screenshots, videos, text, etc)

0.26.1

[root@node1 ztest]# ./zarf_v0.26.1_Linux_amd64 package create --confirm

Using config file /root/ztest/zarf-config.toml

Saving log file to /tmp/zarf-2023-05-26-12-47-06-857083603.log

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

kind: ZarfPackageConfig
metadata:
  name: test-wrapper
  description: test outer wrapper
components:
- name: test-component
  required: true
  actions:
    onCreate:
      before:
      - cmd: tar czvf file.tar.gz -C "import" file.txt
        description: Process that ends up with a tar.gz
      after:
      - cmd: rm -rf file.tar.gz
        description: Cleans up
  files:
  - source: import/../file.tar.gz
    target: /tmp/remote_file.tar.gz
  ✔  Create Zarf package confirmed

                                                                                       
  📦 TEST-COMPONENT COMPONENT                                                          
                                                                                       

     file.txt                                                                                                                                                                                                                                                    
  ✔  Completed "Process that ends up with a tar.gz"                                                                                                                                                                                                              
  ✔  Completed "Cleans up"                                                                                                                                                                                                                                       
  ✔  Creating SBOMs for 0 images and 1 components with files.                                                                                                                                                                                                    

0.27.0

[root@node1 ztest]# ./zarf_v0.27.0_Linux_amd64 package create --confirm

Using config file /root/ztest/zarf-config.toml

Saving log file to /tmp/zarf-2023-05-26-12-47-20-3120995904.log
     ERROR:  Failed to create package: unable to compose component test-component: unable to get child component:
             unable to fix composed filepaths: imported path import/###ZARF_PKG_TMPL_RELATIVE_DIR###/file.tar.gz
             does not exist

Severity/Priority

Additional Context

I know that this would work if I collapsed it into the wrapper. However, in the actual usage of this, we like to have a separate package created as well that doesn't include the other parts that get added to the outer wrapper that mainly imports. So this is a behavioral change and I am not certain if it is a bug or not but it also might be a teaching moment since maybe I am going about it wrong, please let me know if there is a better way. Bottom line, would like to have an imported zarf package create something that didn't previously exist and still have it be copied properly honoring any "before" actions that should take place before the files are looked at.

I have attached the setup for easy recreation.
zarf_test.tar.gz

@Noxsios
Copy link
Contributor

Noxsios commented May 27, 2023

You are correct @WeaponX314 . Before 0.27 you would have to use path hacking and variables (like your example) in order to use certain from imported components. In 0.27 this is no longer the case as actions and all imported resources are now properly relatively pathed from their source zarf.yaml. The following is how this behavior should work:

kind: ZarfPackageConfig
metadata:
  name: test-wrapper
  description: "test outer wrapper"

components:
  - name: test-component
    required: true
    import:
      path: import
      name: my-import-test

and

kind: ZarfPackageConfig
metadata:
 name: test-pkg
 description: "test creating something before copy"

components:
 - name: my-import-test
   actions:
     onCreate:
       before:
         - cmd: tar czvf file.tar.gz file.txt
           description: Process that ends up with a tar.gz
       after:
         - cmd: rm -rf file.tar.gz
           description: Cleans up
   files:
     - source: file.tar.gz
       target: /tmp/remote_file.tar.gz

However, you have properly identified that onCreate.before actions are run after import composition has run. During composition the filepaths of all resources are checked, and your example errors because the filepath is created after it is checked.

There are 2 paths forward I will bring up to the team:

  1. Disable path checking during composition and wait for the copy operation to fail during create
  2. Run onCreate.before actions during composition and ignore duplication of that action during create

@Noxsios Noxsios added bug 🐞 Something isn't working and removed possible-bug 🐛 labels May 27, 2023
@Racer159
Copy link
Contributor

Racer159 commented May 30, 2023

I personally would advocate for # 1 in this case. It would be a faster fix and would have less of a chance to break other behavior. I also updated the release notes to be more explicit about the change in behavior here.

@Noxsios Noxsios linked a pull request May 30, 2023 that will close this issue
5 tasks
@Racer159 Racer159 added this to the v0.27 (m3-5.30) milestone May 31, 2023
Noxsios added a commit that referenced this issue Jun 1, 2023
## Related Issue

Fixes #1756 

## 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

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

---------

Signed-off-by: razzle <harry@razzle.cloud>
Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com>
Co-authored-by: Wayne Starr <me@racer159.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.

3 participants