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

Add format tar.gz #277

Merged
merged 24 commits into from
Aug 6, 2024
Merged

Add format tar.gz #277

merged 24 commits into from
Aug 6, 2024

Conversation

jkroepke
Copy link
Contributor

@jkroepke jkroepke commented Nov 22, 2023

In case you need this feature as well, please do a community vote with 👍


Ref: #272, #29

Fixes: #241

This PR adds tar.gz format. I only add tar.gz here because I hope its easier to review compared to other PRs.
I think, additional formats can be setup in different PRs. tar.gz is fully covered by go stdlib. No external libraries are required.

Internally, the NewTarArchiver can be extended with additional compression methods. At the moment, only gzip is implemented.


Fork with tar.gz support: https://registry.terraform.io/providers/jkroepke/archive/2.4.2+tar.gz

@jkroepke jkroepke requested a review from a team as a code owner November 22, 2023 07:50
@jkroepke
Copy link
Contributor Author

@bendbennett Let me know, if I can assists here on review

@bendbennett
Copy link
Contributor

Hi @jkroepke 👋
Thank you for your submission and for linking to a related PR and issue.

Looking at the issue for Support and array of compression formats and specifically this comment, I think we would need to ascertain whether we should embrace extending the surface area of functionality of the archive provider in this way. I have marked this PR for triage so that this can be discussed at our next triage meeting. Another consideration that we need to weigh up is the level of community interest. This could either be gauged through the number of upvotes on this PR, or if you felt inclined to, you could create an accompanying issue to outline the use-case(s) for adding .tar.gz compression. Upvotes on the accompanying issue could then also be used to gauge the level of community interest.

Thanks again for the submission.

@jkroepke
Copy link
Contributor Author

jkroepke commented Nov 23, 2023

Hi @bendbennett

thanks for the response.

An issue for tar.gz already exists #241. In my case, its exactly the same reason. I have to upload data to an Azure Storage Account / S3 Bucket. zip does not fit here, because the command unzip is not installed by default on all Linux System. (including Ubuntu). tar.gz is supported by default on Windows and Linux.

The I read #29 and understand that this module was designed for AWS Lamba. However I would like to use is to distribute Software via S3 buckets.

@jkroepke
Copy link
Contributor Author

jkroepke commented Dec 4, 2023

Hi @bendbennett do you have an update here?

@bendbennett
Copy link
Contributor

Hi @jkroepke 👋

I'm afraid I don't have an update at this time. I will raise this PR for discussion at our next triage meeting, and I will provide some feedback once we have had a chance to discuss your PR as a team. Thank you for your contribution.

@ashwin153
Copy link

@bendbennett Cloud Deploy expects your Skaffold configuration to be stored in a tar.gz archive in Cloud Storage. I would like to create this archive and upload it to GCS using Terraform, but I’m currently not able to do that without resorting to null_resource / local_file hacks. Would you be willing to reconsider inclusion of tar.gz in this provider?

@ashwin153
Copy link

@jkroepke Would you be able to sync your fork and publish it to the Terraform registry until this pull request lands?

@jkroepke
Copy link
Contributor Author

jkroepke commented Feb 4, 2024

Let’s wait a week if @bendbennett respond. Otherwise @ashwin153 pls ping here in a week again.

@ashwin153
Copy link

@jkroepke Looks like no response on this.

@jkroepke
Copy link
Contributor Author

https://registry.terraform.io/providers/jkroepke/archive/2.4.2+tar.gz just publish as-is

@jkroepke
Copy link
Contributor Author

@bendbennett do you have any new for us?

@bendbennett
Copy link
Contributor

Hi @jkroepke 👋

Looking at this issue, it appears that there are currently 2 up-votes for adding the functionality in the PR you have submitted. As mentioned above, we need to weigh-up increasing the surface area of the utility providers (e.g., archive provider) against the level of community interest. At this time, it does appear that the community does not seem to be expressing a high level of interest in adding tar.gz as an output format. We're happy to keep the PR open and continue to monitor the level of community interest, and will re-visit this issue if interest in this change increases. Thank you.

@gr4ytech
Copy link

Hi there! Just wanted to pitch in. We're configuring Palo firewalls and need to use tar.gz archives to inject the initial bootstrapping metadata. It would be very helpful to have these archives available as there are variables we need to change in the initial xml surrounding interface ips. This would significantly speed up our workflow. Thanks!

@jkroepke
Copy link
Contributor Author

Hey @bendbennett

I observe more votes on this PR. Is it enough to move forward here? Some users share additional use-cases as well.

@wanpdsantos
Copy link

I have a different use case. For AWS Service Catalog products, if you want to use TERRAFORM_TEMPLATE product type we need the tar.gz file. This PR will be very helpful in this case.

@jkroepke
Copy link
Contributor Author

Hey @bendbennett

could you take a look here again please? There is a significant community interest.

@qmugnier
Copy link

This feature would be actually very valuable for instance when you want to "copy" files (actually write files) with cloud init as it is currently limited to 16k.

@deniseyu
Copy link

Hi @jkroepke 👋🏼 Thanks for your contribution here.

I'm the new manager of the team, so I'll do my best to help, but please be patient with me as I'm still ramping up.

A lot of new capabilities have shipped within Terraform Core and among the most-used providers since ~2018, when we first started discussing support for additional compression types. We're going to do an audit of all of our utility providers this summer to determine which ones are the most impactful for us to continue to officially maintain. I'm sorry I can't give you a timeline on this, but we are taking into account all of the feedback from the community in this issue, as well as the related ones.

To that end, we're pausing additional feature development on this provider for now. Thanks for your patience.

@oboukili
Copy link

Hate to be that guy but, this is a finalized PR with extensive tests many of us have been eyeing for a very long time, and have been providing very relevant usefulness examples.
It took only a few weeks to get a substantial increase in upvotes, demonstrating the impact potential. I understand you wish to reduce the amount of work you’d have to migrate over TF core, but surely this PR could make it just in time before the official deprecation notice for us to benefit from this today. Thanks for your consideration 🙏!

@jkroepke
Copy link
Contributor Author

In case, they discontinue the support for this provider, I consider the fork and continue the maintenance here.

@qmugnier
Copy link

qmugnier commented May 1, 2024

@jkroepke : would it be possible to upload your tar branch to the opentofu registry? With all the things going on with the license change and the Hashicorp buyout, we are migrating to opentofu but your provider is not available there.

@jkroepke
Copy link
Contributor Author

jkroepke commented May 1, 2024

@jkroepke : would it be possible to upload your tar branch to the opentofu registry? With all the things going on with the license change and the Hashicorp buyout, we are migrating to opentofu but your provider is not available there.

opentofu/registry#430

@ashwin153
Copy link

@jkroepke Any thoughts on my review comments?

jkroepke and others added 13 commits August 5, 2024 19:07
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Co-authored-by: Selena Goods <github@simplebox.anonaddy.com>
Co-authored-by: Selena Goods <github@simplebox.anonaddy.com>
…linkDirectories and TestTarArchiver_Dir_ExcludeSymlinkDirectories
@jkroepke
Copy link
Contributor Author

jkroepke commented Aug 5, 2024

@SBGoods I guess it should meet your expectations

Copy link
Contributor

@SBGoods SBGoods left a comment

Choose a reason for hiding this comment

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

Thank you for revising the PR @jkroepke! I just requested one minor change to remove extra test runs. The tests are all passing in the draft PR so the PR should be good to go after this change.

internal/provider/data_source_archive_file_tgz_test.go Outdated Show resolved Hide resolved
@jkroepke
Copy link
Contributor Author

jkroepke commented Aug 5, 2024

Done 👍

@jkroepke jkroepke requested a review from SBGoods August 6, 2024 13:31
Copy link
Contributor

@SBGoods SBGoods left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you again for the contribution @jkroepke!

@SBGoods SBGoods added this to the v2.6.0 milestone Aug 6, 2024
@SBGoods SBGoods added the feature label Aug 6, 2024
@SBGoods SBGoods merged commit 7708696 into hashicorp:main Aug 6, 2024
5 checks passed
@jkroepke jkroepke deleted the tar.gz branch August 6, 2024 21:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Additional Compression Types(Ex: tar.gz format)
10 participants