Skip to content

add support for installing released plugins#1658

Merged
pkwarren merged 4 commits intomainfrom
pkw/BSR-903-install-pre-built
Dec 7, 2022
Merged

add support for installing released plugins#1658
pkwarren merged 4 commits intomainfrom
pkw/BSR-903-install-pre-built

Conversation

@pkwarren
Copy link
Member

@pkwarren pkwarren commented Dec 7, 2022

Update buf alpha plugin push command to add support for installing plugins from released versions (zip file containing buf.plugin.yaml and image.tar). Remove all support for building Docker images (Docker CLI is preferred).

Update `buf alpha plugin push` command to add support for installing
plugins from released versions (zip file containing buf.plugin.yaml and
image.tar). Remove all support for building Docker images (Docker CLI is
preferred).
@pkwarren pkwarren requested a review from mfridman December 7, 2022 20:32
return err
}
var sourceBucket storage.ReadWriteBucket
if !sourceStat.IsDir() && strings.HasSuffix(strings.ToLower(sourceStat.Name()), ".zip") {
Copy link
Member Author

Choose a reason for hiding this comment

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

If argument is a zip file, then we'll create a temp dir and unpack contents to it.

tagResponse, err := client.Tag(ctx, flags.Image, pluginConfig)
var imageToTag string
if flags.Image != "" {
imageToTag = flags.Image
Copy link
Member Author

Choose a reason for hiding this comment

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

The --image always takes precedence. If not specified, we fall back to looking for a pre-built image.tar in the directory where the buf.plugin.yaml is found.

Copy link
Member

Choose a reason for hiding this comment

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

I'd error to avoid surprising behavior. But we don't need to get the UX ideal right now, let's see how this will be used.

}

func buildDockerPlugin(t testing.TB, dockerClient Client, dockerfilePath string, pluginIdentity string, pluginVersion string) (*BuildResponse, error) {
func buildDockerPlugin(t testing.TB, dockerfilePath string, pluginIdentity string) (string, error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed code which used to build docker images in favor of just shelling out to the docker command to run some basic tests. This still allows us to keep coverage up for issues seen when pushing images, etc.

Copy link
Member

Choose a reason for hiding this comment

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

At this point we're effectively testing the docker cli, do we still find these tests useful?

cli, err := client.NewClientWithOpts(dockerClientOpts...)
if err != nil {
return nil, err
}

Did the previous tests require you to have docker cli or the docker engine installed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah they did. We still exercise tests where we simulate errors from the docker registry push, etc. I think it is still useful to keep around.

github.com/jhump/protoreflect v1.14.0
github.com/klauspost/compress v1.15.12
github.com/klauspost/pgzip v1.2.5
github.com/moby/buildkit v0.10.5
Copy link
Member

Choose a reason for hiding this comment

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

😌

@pkwarren pkwarren merged commit acd74fb into main Dec 7, 2022
@pkwarren pkwarren deleted the pkw/BSR-903-install-pre-built branch December 7, 2022 22:48
Monirul1 pushed a commit to Monirul1/buf that referenced this pull request Apr 30, 2023
Update `buf alpha plugin push` command to add support for installing
plugins from released versions (zip file containing buf.plugin.yaml and
image.tar). Remove all support for building Docker images (Docker CLI is
preferred).
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 this pull request may close these issues.

2 participants