Add missing autobuild-disabled flag to publish command

[alekseybb197]

What does this change

Added cli flag and configuration key for prevent unexpected autobuild invocation image during publish command because it crush pipelines execute.

Use as "porter publish ... --autobuild-disable" in cli or insert "autobuild-disable: true" into config.yaml.

What issue does it fix

Follow-up to #2573 which missed adding the flag to publish

Notes for the reviewer

Checklist

• Did you write tests?
• Did you write documentation?
• Did you change porter.yaml or a storage document record? Update the corresponding schema file.
• If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

Reviewer Checklist

• Comment with /azp run test-porter-release if a magefile or build script was modified
• Comment with /azp run porter-integration if it's a non-trivial PR

[carolynvs]

Closing as this was already implemented in #2573. You can find more open issues that are good for new contributors at https://getporter.org/find-issue

[alekseybb197]

Sorry, as I saw this flag is absent into publish code! And it crush our pipelines which build and publish cnab. Because we can not able send build-arg when autobuild appeared. But let it be)

[carolynvs]

Oops, thanks for pushing back, I thought I had updated the publish command in that pull request and see now that it was missed. I'll reopen and review.

[carolynvs]

@alekseybb197 Thank you for the fix! I have pushed 2 commits to your pull request to simplify how --autobuild-disabled is defined on publish and added a test to validate that the flag is working.

Before I can merge, can you push a commit to your branch that includes the following text

Signed-off-by: Aleksey Barabanov <[email protected]>

This indicates you agree to our repository's DCO requirement. https://getporter.org/contribute/guide/#signing-your-commits

[carolynvs]

/azp run porter-integration

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[alekseybb197]

Thanks @carolynvs , I try ...
I was upset and merged as is for my fork)

[alekseybb197]

@carolynvs I see you remove "autobuild-disabled" from config file. Is it right? I'm only asking about the accepted decision. In other words, I do not need to return it with my commit?

[carolynvs]

Correct, I removed AutoBuildDisabled from datastore.go because it is not necessary. The additional viper configuration in the cmd/porter package and declaring a config setting in DataStore is only needed for configuration values that are not defined as flags on a command. So please do not add it back.

You can validate this locally by running mage build install and then editing your ~/.porter/config.yaml file to set autobuild-disabled: true. Then try to publish a bundle without building it first and you should see it stop immediately because autobuild is disabled.

[alekseybb197]

Carolyn, with all due respect, I can't do this.

ansible@test-vm:/local$ git clone https://github.com/getporter/porter.git
ansible@test-vm:/local$ cd porter
ansible@test-vm:/local/porter$ mage build
ansible@test-vm:/local/porter$ LC_ALL=C ll bin
total 62088
drwxrwx--- 5 ansible ansible 4096 Apr 19 23:33 ./
drwxrwxr-x 16 ansible ansible 4096 Apr 19 23:33 ../
drwxrwx--- 9 ansible ansible 4096 Apr 19 23:33 mixins/
-rwxrwxr-x 1 ansible ansible 63555130 Apr 19 23:33 porter*
drwxrwx--- 2 ansible ansible 4096 Apr 19 23:33 runtimes/
drwxrwx--- 2 ansible ansible 4096 Apr 19 23:33 v1.0.13-14-gc0dcb287/
ansible@test-vm:/local/porter$
ansible@test-vm:/local/porter$ mage install
installing Porter from bin to /home/ansible/.porter
ansible@test-vm:/local/porter$ echo "autobuild-disabled: true" >/.porter/config.yaml
ansible@test-vm:/local/porter$ cd ../examples/helm-multiarch/
ansible@test-vm:/local/examples/helm-multiarch$ porter publish -r far.far.away
Changes have been detected and the previously built bundle is out-of-date, rebuilding the bundle before proceeding...
Building bundle ===>
Copying porter runtime ===>
Copying mixins ===>
Copying mixin exec ===>
Copying mixin helm3 ===>
Building invocation image
[+] Building 1.9s (5/5)
=> [internal] load build definition from Dockerfile 0.0s
=> => transferring dockerfile: 1.26kB 0.0s
=> [internal] load .dockerignore 0.0s
=> => transferring context: 227B 0.0s
=> resolve image config for docker.io/docker/dockerfile-upstream:1.4.0 1.0s
=> CACHED docker-image:https://docker.io/docker/dockerfile-upstream:1.4.0@sha256:178c4e4a93795b9365dbf6cf10da8fcf517fcb4a17f1943a775c0f548e9fc2ff 0.0s
=> [internal] load metadata for docker.io/library/debian:stretch-slim 0.7s
^Ccancel requested

Maybe I'm wrong about something. Maybe I'm missing something

[carolynvs]

Let me double check locally. I can also update the test to explicitly use config instead of the flag.

[carolynvs]

I found a regression in binding config file values to flags in some circumstances. #2735

Once that is fixed (in-progress), then the autobuild-disabled config file setting should work as expected without the additional configuration (defining it on DataStore).

[alekseybb197]

Great! But on the other hand, I don’t know how to fill a commit with a signature :)
I admit it's my fault that I didn't set up dco in time

[carolynvs]

One of your commits does have a signature, so I have marked the DCO as passing. When I merge, I will squash the commits into a single signed commit, co-authored by both of us. So everything is fine.

I should have that config file regression fixed soon. Once that's merged, I'll update my publish test to use a config file (instead of a flag), and we can be sure that the original issue that you raised is addressed before I merge this PR.

[alekseybb197]

Thanks a lot Carolyn! Have a nice day!

[carolynvs]

/azp run porter-integration

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[carolynvs]

Thank you for your patience on this one! I appreciate the bug report and fix 👍

[carolynvs]

🎉