Skip to content
This repository has been archived by the owner on Nov 25, 2024. It is now read-only.

Add yq As a Tool #249

Merged
merged 1 commit into from
Jan 17, 2024
Merged

Add yq As a Tool #249

merged 1 commit into from
Jan 17, 2024

Conversation

ulucinar
Copy link
Contributor

@ulucinar ulucinar commented Jan 16, 2024

Description of your changes

We are planning to use kustomize to process the family provider CRDs. kustomize outputs a multi-document YAML that contains all the processed CRDs and the up xpkg batch command relies on the controller-tools naming conventions for the CRD files to decide which CRD goes into which family provider package. Thus it cannot currently partition CRDs if they are in a single file. I checked the possibility of a content based filter (instead of relying on the generated file name) but the interface is io.ReadCloser, which processes a byte stream and is not aware of the YAML documents. Implementing a content filter will introduce unnecessary complexity due to the mismatching abstraction layers. Also considered a filtering parser.Parser implementation but that interface returns a concrete parser.Package (struct), which does not allow writes to its content list. So, that approach would require us to change the parser.Parser interface.

Instead of introducing yq, we first implemented a bash script processing the multi-doc YAML file line-by-line and splitting the over 900 CRDs of the official AWS provider family and it takes ~70 s on an M2 Mac to do so. Doing experiments with yq, it completes in ~10 s. So, we would like to use yq for splitting the multi-doc YAML output from kustomize.

I have:

  • Read and followed Upbound's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR, as appropriate.

How has this code been tested

Tested both on the darwin ARM64 & linux AMD64 platforms.

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Copy link
Member

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

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

Thanks @ulucinar LGTM!

Copy link
Member

@turkenh turkenh left a comment

Choose a reason for hiding this comment

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

Thanks @ulucinar 🙌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants