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

Discuss: Move to zip instead of tar.gz for the format of the package. #474

Closed
ph opened this issue May 18, 2020 · 15 comments · Fixed by #628
Closed

Discuss: Move to zip instead of tar.gz for the format of the package. #474

ph opened this issue May 18, 2020 · 15 comments · Fixed by #628
Assignees

Comments

@ph
Copy link

ph commented May 18, 2020

Currently, the package registry uses the tar.gz as the compressed package format for shipping the packages down to Kibana. Sadly this format is not supported directly by Windows. It would be nice if we would move to a zip file format this would allow anyone to download the package on windows unzip it and inspect the content.

@ruflin I vaguely remember you mentioning that there were some issues in Kibana that prevent us from moving to ZIP.

@ruflin
Copy link
Contributor

ruflin commented May 19, 2020

We had zip support in the past but it was removed here: #59 @jfsiii might chime in what works better and why on the Kibana side.

With the work @mtojek is doing on #472 it will be pretty simple to support both. If we support both, we need to discuss if we should adjust the /search API output:

Example:

  {
    "description": "aws Integration",
    "download": "/epr/aws/aws-0.0.3.tar.gz",
    "icons": [
      {
        "src": "/package/aws/0.0.3/img/logo_aws.svg",
        "title": "logo aws",
        "size": "32x32",
        "type": "image/svg+xml"
      }
    ],
    "name": "aws",
    "path": "/package/aws/0.0.3",
    "title": "aws",
    "type": "integration",
    "version": "0.0.3"
  },

@jfsiii
Copy link

jfsiii commented Jun 2, 2020

I'd like to support both/multiple. tar.gz makes things easier on the Manager if it's on darwin or linux. .zip would be great for those on Windows.

I'm not sure about the specific API. Some options that come to mind are:

  • add a query param like archive_format=zip with a default and the download key would remain a string.
  • move to an array of objects like icons

@ruflin
Copy link
Contributor

ruflin commented Jun 2, 2020

@jfsiii If you are thinking of an array, are you thinking of one with objects inside? If we go this path, it would allow us to add "metadata" to each download link potentially if needed so Kibana for example does not have to take the suffix to decide which download link to use but could do it on a specific field that tells it, it is a tar.gz.

ruflin added a commit to ruflin/package-registry that referenced this issue Jun 15, 2020
As discussed in elastic#474 in the future we might have more then one format to download a package. To be prepared for this, downloads becomes an array.
@ruflin
Copy link
Contributor

ruflin commented Jun 15, 2020

I opened #512 as a draft to get some initial feedback.

@ruflin ruflin added the Ingest Management:beta1 Group issues for ingest management beta1 label Jun 22, 2020
ruflin added a commit to ruflin/package-registry that referenced this issue Jun 22, 2020
As discussed in elastic#474 in the future we might have more then one format to download a package. To be prepared for this, downloads becomes an array.

In this PR, the existing `download` entry is not removed yet. It will be removed as soon as Kibana switched over the usage.
ruflin added a commit to ruflin/package-registry that referenced this issue Jun 24, 2020
As discussed in elastic#474 in the future we might have more then one format to download a package. To be prepared for this, downloads becomes an array.

In this PR, the existing `download` entry is not removed yet. It will be removed as soon as Kibana switched over the usage.
ruflin added a commit to ruflin/package-registry that referenced this issue Jun 24, 2020
As discussed in elastic#474 in the future we might have more then one format to download a package. To be prepared for this, downloads becomes an array.

In this PR, the existing `download` entry is not removed yet. It will be removed as soon as Kibana switched over the usage.
ruflin added a commit that referenced this issue Jun 24, 2020
As discussed in #474 in the future we might have more then one format to download a package. To be prepared for this, downloads becomes an array.

In this PR, the existing `download` entry is not removed yet. It will be removed as soon as Kibana switched over the usage.
@roncohen
Copy link

roncohen commented Jul 2, 2020

we should only support one format. We agreed that unless there's big complications, it should be ZIP. It's natively support in MacOS and Windows and Linux users will be able to manage.

@ph
Copy link
Author

ph commented Jul 2, 2020

Adding this to b2, we need to do this before we go out of beta.

@ruflin
Copy link
Contributor

ruflin commented Jul 3, 2020

I opened a PR to remove the list again as we didn't use it yet: #584 If we move to zip, will need to figure out a non breaking way in b2, but should be doable :-)

@ruflin ruflin added Ingest Management: beta2 and removed Ingest Management:beta1 Group issues for ingest management beta1 labels Jul 3, 2020
@jfsiii
Copy link

jfsiii commented Aug 20, 2020

@ruflin pinging since this is targeting 7.10. I don't think it'll be a big deal for Kibana, but LMK when there is a PR/Docker image I can use to test

@ruflin
Copy link
Contributor

ruflin commented Aug 21, 2020

@jfsiii Can you forsee any platform / streaming specific challenges around decompression zip vs tar.gz in Kibana? Will ping you as soon as this moves forward. We need to first sort out a separate registry for 7.10 as this will be breaking.

@jfsiii
Copy link

jfsiii commented Aug 26, 2020

@ruflin writing down what I said to you on Monday, I think we'll be ok. We used to support both. We currently only deal with it in one place, where we decompress it into a Buffer. I think we can replace untar* with unzip* in one place and be good

https://github.com/elastic/kibana/blob/b4b6428c1ca095f8e1388bcec29d8d508ec08a22/x-pack/plugins/ingest_manager/server/services/epm/registry/index.ts#L178-L187

@ph
Copy link
Author

ph commented Aug 26, 2020

@skh for awareness with your work for installing a package without the registry.

@skh
Copy link
Contributor

skh commented Aug 27, 2020

We used to support both.

On the Kibana/EPM side I'd like to suggest that we support both again, at least for a transitional period until all registries are safely switched over.

ruflin added a commit to ruflin/package-registry that referenced this issue Aug 27, 2020
This is a breaking change and first needs changes on the Kibana side. The new registry and the new version of Kibana should be either rolled out in sync or Kibana supports both for a bit.

Closes elastic#474
ruflin added a commit to ruflin/package-registry that referenced this issue Aug 27, 2020
This is a breaking change and first needs changes on the Kibana side. The new registry and the new version of Kibana should be either rolled out in sync or Kibana supports both for a bit.

Closes elastic#474
@ruflin
Copy link
Contributor

ruflin commented Aug 27, 2020

I opened #628 with the change. @jfsiii Could you take a stab at the change? If it is "simple" I'm ok to support both in Kibana for a few days so we can roll out everything properly and things keep working.

@jfsiii
Copy link

jfsiii commented Aug 27, 2020

@ruflin @skh I put my first pass at it in as a draft PR elastic/kibana#76197

@ruflin
Copy link
Contributor

ruflin commented Aug 31, 2020

@jfsiii Great, did you manage to test this with the changed registry?

ruflin added a commit to ruflin/package-registry that referenced this issue Sep 3, 2020
This is a breaking change and first needs changes on the Kibana side. The new registry and the new version of Kibana should be either rolled out in sync or Kibana supports both for a bit.

Closes elastic#474
ruflin added a commit to ruflin/package-registry that referenced this issue Sep 7, 2020
This is a breaking change and first needs changes on the Kibana side. The new registry and the new version of Kibana should be either rolled out in sync or Kibana supports both for a bit.

Closes elastic#474
ruflin added a commit to ruflin/package-registry that referenced this issue Sep 8, 2020
This is a breaking change and first needs changes on the Kibana side. The new registry and the new version of Kibana should be either rolled out in sync or Kibana supports both for a bit.

Closes elastic#474
ruflin added a commit that referenced this issue Sep 8, 2020
This is a breaking change and first needs changes on the Kibana side. The new registry and the new version of Kibana should be either rolled out in sync or Kibana supports both for a bit.

Closes #474
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants