这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@mihai-stancu
Copy link
Contributor

Based on #354 I've migrated all the phar-download commands to phive-install commands.

Some didn't work tho, idk why.

Note: if the tool was installed with a specific version in the url, then I retained the version specification in the install command (alias@^version). Most tools likely didn't have a latest url setup so the version is probably unnecessary there (it's not a BC break) but I can't know that.

@jakzal
Copy link
Owner

jakzal commented Mar 3, 2021

Note: if the tool was installed with a specific version in the url, then I retained the version specification in the install command (alias@^version). Most tools likely didn't have a latest url setup so the version is probably unnecessary there (it's not a BC break) but I can't know that.

Can we remove the version where possible?

@jakzal
Copy link
Owner

jakzal commented Mar 3, 2021

Some installations fail due to the question about key import:

Downloading https://github.com/mihaeu/dephpend/releases/download/0.7.0/dephpend-0.7.0.phar.asc

Downloading key 76835C9464877BDD
Trying to connect to keys.openpgp.org (37.218.245.50)
Downloading https://keys.openpgp.org/pks/lookup?op=get&options=mr&search=0x76835C9464877BDD

 ╰|>                                       | 0.00 KB / 3.10 KB -   0%
 ╰|========================================>| 3.10 KB / 3.10 KB - 100%
Successfully downloaded key.

Error:    Needs tty to be able to confirm
	Fingerprint: 44CC 65DC 01D2 FC05 AD6F 3DBD 7683 5C94 6487 7BDD

	Michael Haeuslmann <michael.haeuslmann@gmail.com>

	Created: 2019-07-14

Import this key? [y|N] 
make: *** [Makefile:33: test-integration] Error 4

@mihai-stancu
Copy link
Contributor Author

I noticed. Does that mean we should always use --trust-gpg-keys because we can't validate them beforehand?

@mihai-stancu
Copy link
Contributor Author

Note: if the tool was installed with a specific version in the url, then I retained the version specification in the install command (alias@^version). Most tools likely didn't have a latest url setup so the version is probably unnecessary there (it's not a BC break) but I can't know that.

Can we remove the version where possible?

I can remove the version constraint but IDK if there are any compatibility issued.

Do you know if phive checks the php version and restricts installs based on that in any way? For example PHPunit5 may not work with PHP8.

Point is, I'm not aware of the limitations each tool has and/or if that version specification was put there on purpose to cover a BC break or if it was there because there was no URL with "latest" available.

@mihai-stancu
Copy link
Contributor Author

Middle-ground could be alias@^<major_version_without_minor_or_build> which should at least cover PHP-level compat.

@mihai-stancu
Copy link
Contributor Author

mihai-stancu commented Mar 3, 2021

@jakzal BTW, IDK during which context phive is executed without any parameters (sometime right after install?).

The setup command should run phive --home %target-dir% and the test command should run phive --version neither of which should output the help section...

This was caused by improper use of phive --home %target_dir%.

@mihai-stancu
Copy link
Contributor Author

mihai-stancu commented Mar 3, 2021

Apparently the topic of auto-accept isn't going to be a fun one: phar-io/phive#69 ... either we echo "Y" | phive and give up on security or we create a local (manually downloaded) list of GPG keys...

@jakzal
Copy link
Owner

jakzal commented Mar 3, 2021

Is it possible to import keys beforehand in an automated way?

@mihai-stancu
Copy link
Contributor Author

mihai-stancu commented Mar 3, 2021

Is it possible to import keys beforehand?

At the begining of that thread they mentioned a "home for the GPG keys" but IDK if it's a concept that exists or if it was a proposal.

At the end of that thread they mentioned a flag you can use to specify a list of keys that are trusted (which means I was using the --trust-gpg-keys all wrong, it was supposed to be an array of key identifiers).

@jakzal phive has a directory in which it keeps a private gpg keyring. We could run the install once manually and obtain the folder that way, commit it, and have it already present when subsequent builds take place. Signature conflicts will have no automatic upgrade path (if a provider changes his sig).

Any automated import is viewed as a security risk.

@mihai-stancu mihai-stancu changed the title Phive tools Phive > Upgrade all phar-download to phive-install Mar 3, 2021
@mihai-stancu
Copy link
Contributor Author

In order to test the rest of the implementation details I've defaulted the implementation to accepting unsigned packages.

@mihai-stancu
Copy link
Contributor Author

@jakzal the current failure is a permission denied on box but I can't see the full command line being executed, IDK where it's coming from. Can you help?

@jakzal
Copy link
Owner

jakzal commented Mar 4, 2021

It seems that box is downloaded to the box directory:

Downloading https://github.com/box-project/box/releases/download/3.11.1/box.phar
Linking /Users/kuba/Projects/toolbox/local/phars/humbug/box-3.11.1.phar to /Users/kuba/Projects/toolbox/local/box/box

By the way, you can install tools locally for easier debugging:

mkdir local
COMPOSER_HOME=$(pwd)/local/composer PATH=$(pwd)"/local:"$(pwd)"/local/composer/vendor/bin:$PATH" bin/toolbox.php install --target-dir ./local --tag pre-installation

you'll then see that box is a directory:

ls local/
_tmp_wrk/     box/          box-legacy*   composer/     gpg/          http-cache/   phars/        phive*        registry.xml

@mihai-stancu
Copy link
Contributor Author

@jakzal the target parameter of phive only accepts/expects a directory phive install -t <target_directory> as far as i can see you cannot specify the filename.

We have at least 3 instances where we artificially alter the tool name (ex.: phpunit versions).

@jakzal
Copy link
Owner

jakzal commented Mar 5, 2021

We need to think how to work around it.

What's the difference between --home and --target?

Perhaps we could install all phive tools to their own directories grouped under phive %target-dir%/phive/tool-name, and then link each one of them to %target-dir%/tool-name. Just thinking out loud as I haven't thought that through yet.

@mihai-stancu
Copy link
Contributor Author

phive has its own home directory where it keeps:

  • gpg -- it's private keyring for accepted key sigs
  • phars -- the actual phar files in the format <name>-<semver>.phar
  • some xml metadata it needs for normal operations
  • a teporary folder where it keeps downloaded stuff before moving them

By default when you install something it will symlink it into the current folder under the short (versionless) name.
By default when you global install something it will symlink it into /usr/local/bin with the short versionless name.

If you specify -t <target> you're specifying the directory in which to place the symlink.

We can work around it by allowing it to generate a symlink somewhere else (in a "clean" directory such as /tools/.phive/some_clean_tmp_dir) and moving it from there to our desired location. Feels kludgy but it'll do the job.

@mihai-stancu
Copy link
Contributor Author

OK, so we have an makeshift solution that works, it installs all tools using phive.

  • It uses echo "y" | phive install ... as a hack to skip the gpg keyring thing
  • It uses '/tmp/'.md5($alias) as a temporary folder name to provisionally install the file and then mv /tmp/<md5>/* ...

Both of the above hacky solutions aren't very comfortable. How should we proceed?

@mihai-stancu
Copy link
Contributor Author

Note: in a finished docker build gpg is absent so running phive after the install doesn't work because it can't verify the signatures.

I'm not sure why the builds work in git actions -- perhaps gpg is still present in the image at build time (before stripping overlays?).

What command can I use to setup a local test of the full build with phpqa resulting in a docker build?

@jakzal
Copy link
Owner

jakzal commented Mar 9, 2021

Note: in a finished docker build gpg is absent so running phive after the install doesn't work because it can't verify the signatures.

I'm not sure why the builds work in git actions -- perhaps gpg is still present in the image at build time (before stripping overlays?).

Note that the phpqa image is not used in toolbox GitHub actions. gpg is probably installed on GitHub's image.

What command can I use to setup a local test of the full build with phpqa resulting in a docker build?

This is going to be tricky. We don't need it often and in general it requires a released version of toolbox. If it gets too hard I'm open to merge your PR here and create a new release.

  1. Clone jakzal/phpqa
  2. Update Dockerfile-debian and/or Dockerfile-alpine to use your toolbox.
  3. Run make generate to regenerate Dockerfiles.
  4. Build selected version with make build-alpine PHP_VERSION=8.0 or make build-latest PHP_VERSION=8.0 or just make build (last one to build all versions).

Regarding the step 2, you'll need to either replace the curl command that download toolbox to download your own version, or replace it with Docker's ADD ... to include your toolbox.

@mihai-stancu
Copy link
Contributor Author

@jakzal what about the other two issues:

  • It uses echo "y" | phive install ... as a hack to skip the gpg keyring thing
  • It uses '/tmp/'.md5($alias) as a temporary folder name to provisionally install the file and then mv /tmp/<md5>/* ...

Do we try addressing/fixing these issues before releasing?

@jakzal
Copy link
Owner

jakzal commented Mar 14, 2021

@mihai-stancu I'll take a look as soon as I have a bit of time. Thank you for working on this again!

@mihai-stancu
Copy link
Contributor Author

I've successfully tested eliminating echo "y" | ....

Steps:

  • install phive locally
  • use it to install all tools which have signatures
  • copy the resulting ~/.phive/gpg folder into the project
  • commit it into the project
  • change docker image to COPY it into /tools/.phive/gpg
  • rebuild image

Now tools will be installable without requiring signature confirmation.

@jakzal
Copy link
Owner

jakzal commented Mar 15, 2021

Is it easy to make a list of keys and hard code it? i.e. download them in Dockerfile upfront (at least for now, later we could think of a command step).

This way we'd verify the initial list of keys that would be installed on the image. Later, if any of the keys changed or a new tool was added, the build would fail and we'd need to verify/accept new keys again. As a result, we wouldn't be accepting keys blindly.

@mihai-stancu
Copy link
Contributor Author

Is it easy to make a list of keys and hard code it? i.e. download them in Dockerfile upfront (at least for now, later we could think of a command step).

Steps to reproduce what I have tried:

  • run phive --home /path/to/phpqa-repo/.phive install <tools>
  • commit the directory /path/to/phpqa-repo/.phive/gpg
  • update the Dockerfile to COPY ./.phive/gpg /tools/.phive/gpg
  • now inside the container running phive --home /tools/.phive install <tools> will not ask for a signature

@mihai-stancu
Copy link
Contributor Author

Another approach would be to store a list of signatures in a txt file and pass them all to --trust-gpg-signature ....

It feels like it's the same approach as above, with extra steps.

@mihai-stancu
Copy link
Contributor Author

This just dawned on me: each command.phive-install.command could have a property signature to execute phive install --trust-gpg-signature $signature`.

IDK how I missed this, this was very obvious.

@jakzal
Copy link
Owner

jakzal commented Mar 18, 2021

Starts looking good 👍

Builds are currently failing due to an issue unrelated to this PR: deprecated-packages/symplify#3056

@mihai-stancu
Copy link
Contributor Author

Did you disable ecs?

That allowed the tests to continue up until they hit phpbench which wasn't working for PHP8.

Now everything works. Yay!

@jakzal
Copy link
Owner

jakzal commented Mar 19, 2021

Did you disable ecs?

No, they seem to have fixed the phar :)

@jakzal
Copy link
Owner

jakzal commented Mar 19, 2021

@mihai-stancu I will make a new toolbox and phpqa releases. This will give you some time to finalise this PR.

Once ready, we'll merge it and release toolbox.

@mihai-stancu
Copy link
Contributor Author

mihai-stancu commented Mar 19, 2021

Status:

  • GPG keys are now handled nicely via key signatures + keyservers
  • phpqa image has been updated with gpg packages, that build is currently failing due to phpstan
  • ugly '/tmp/'.md5($alias) has not been addressed -- do you have any ideas?

@jakzal
Copy link
Owner

jakzal commented Mar 22, 2021

@mihai-stancu can you rebase again please?

@jakzal
Copy link
Owner

jakzal commented Mar 22, 2021

Also, seems we can't be using psalm as phar to use psalm plugins (see #252). Can you revert it back please?

@jakzal
Copy link
Owner

jakzal commented Mar 22, 2021

I pushed 994f20a to your branch. This is an idea to work around the ugly /tmp usage. It also makes that phive files and directories are not polluting the /tools/ directory but are nicely contained in /tools/.phive. WDYT?

As a result, we don't use the "bin part" of the bin option:

"command": {
    "phive-install": {
        "alias": "dephpend",
        "bin": "%target-dir%/dephpend",
        "sig": "76835C9464877BDD"
    }
}

How about we turn it into a target (or whatever phive is calling it)?

"command": {
    "phive-install": {
        "alias": "dephpend",
        "target": "%target-dir%",
        "sig": "76835C9464877BDD"
    }
}

@mihai-stancu
Copy link
Contributor Author

Rebased. Checks passed.

@jakzal
Copy link
Owner

jakzal commented Mar 22, 2021

Outstanding work! Thank you @mihai-stancu 🍻

@jakzal jakzal merged commit 4cc18c3 into jakzal:master Mar 22, 2021
@mihai-stancu
Copy link
Contributor Author

\m/ (-_-) \m/

🍻

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