hashicorp / terraform-provider-archive

Utility provider that provides a data source that can create zip archives for individual files or collections of files.
https://registry.terraform.io/providers/hashicorp/archive/latest/docs
Mozilla Public License 2.0
89 stars 62 forks source link

Add format tar.gz #277

Closed jkroepke closed 1 month ago

jkroepke commented 9 months ago

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 commented 9 months ago

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

bendbennett commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

Hi @bendbennett do you have an update here?

bendbennett commented 9 months ago

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 commented 7 months ago

@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 commented 7 months ago

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

jkroepke commented 7 months ago

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

ashwin153 commented 7 months ago

@jkroepke Looks like no response on this.

jkroepke commented 7 months ago

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

jkroepke commented 6 months ago

@bendbennett do you have any new for us?

bendbennett commented 6 months ago

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 commented 6 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 4 months ago

Hey @bendbennett

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

qmugnier commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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

qmugnier commented 4 months ago

@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 commented 4 months ago

@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.

https://github.com/opentofu/registry/pull/430

ashwin153 commented 4 months ago

@jkroepke Any thoughts on my review comments?

jkroepke commented 3 months ago

@deniseyu do you have an update for us?

jkroepke commented 2 months ago

@austinvalle @deniseyu do you have an update for us?

jkroepke commented 1 month ago

Great. new linters are introduced, i will look into them.

jkroepke commented 1 month ago

@SBGoods please approve CI run.

SBGoods commented 1 month ago

Hi @jkroepke, I apologize for all the CI weirdness. Our Github actions test workflow uses configuration variables and due to an quirk with Github, PRs opened from forked branches do not inherit the variables. Because of that, your PR has not been running all of the proper CI acceptance test checks. I've opened a draft PR based on your branch in #353 in order to run the CI. I will try to keep the draft PR updated with this branch.

jkroepke commented 1 month ago

Thanks for looking into this. Looked at the acctests on the forked PR.

I run the tests local and found additional issues now. I will write an update here once I fixed all tests.

jkroepke commented 1 month ago

Tests are green now - at least on my machine.

Please let me know, if the currently code is fine. After review, but before merge, I would like to do some real world testing.

jkroepke commented 1 month ago

Windows, alright. I have access to an windows dev machine. I will look into that.

jkroepke commented 1 month ago

@SBGoods I found the issue on Windows, tests are green on my local windows system.

jkroepke commented 1 month ago

@SBGoods I guess it should meet your expectations

jkroepke commented 1 month ago

Done πŸ‘