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

"Latest" install folder isn't always the "latest" #46

Closed
marcelais opened this issue Apr 26, 2022 · 13 comments
Closed

"Latest" install folder isn't always the "latest" #46

marcelais opened this issue Apr 26, 2022 · 13 comments
Labels
bug Something isn't working question Further information is requested

Comments

@marcelais
Copy link

(All version numbers are examples.) If you install v0.2.0 of the AuthTool and then subsequently install v0.1.0, the 'latest' folder will point at the v0.1.0 version and not the v0.2.0 version.

Using the latest folder is great for not having to update config files when a new version is released, but if that folder can "regress" to an easlier version, it's usefullness is more limited.

@kyle-rader kyle-rader added bug Something isn't working question Further information is requested labels Apr 26, 2022
@reillysiemens
Copy link
Member

Thanks for reporting this, @marcelais. Unfortunately, at the moment we don't have any great ideas for a solution that don't involve teaching the installation scripts how to understand SemVer, which we aren't keen on at the moment.

The most likely path forward will be for us to pursue proper package management for the various supported platforms (e.g. winget packages, .deb/.rpm packages, etc.), but we don't have an ETA for that just yet.

For now, I'd like to acknowledge this issue by leaving it open and invite any suggestions for a good cross-platform solution.

@kyle-rader
Copy link
Contributor

I think this bug is more important if we want to enable other applications to be able to install the CLI using the scripts. I was just thinking through using the scripts in a library wrapper, and we wouldn't want App 1 to install and use 0.3.0, another app installs 0.2.0 and reset latest back, at least internally this could confuse things.

@kyle-rader
Copy link
Contributor

kyle-rader commented Jun 10, 2022

@marcelais , @AzureAD/security-experience , @reillysiemens and I have talked through how to change the installation to avoid thrashing on the version latest is pointing to.

We think we want to achieve

  • The ability to install yourself, and have azureauth on the PATH.
  • The ability to install a version, and not update the PATH
  • Avoid any SemVer comparison in the install scripts.

Versioned installs only (no more latest):

Main logic:

  • clean install X (default): installs X, and replaces current version on PATH with X.
  • clean install X (no-path): install X.

Example Scenarios

Clean Install 3 (default)

  • install version 3.
  • add version 3 to PATH.

Clean Install 3 (no-path)

  • install version 3

Dirty Install 4 (default) (with 3 on PATH)

  • install version 4
  • replace version 3 with 4 in PATH

Dirty Install 4 (no-path) (with 3 on PATH)

  • install version 4

Dirty Install 2 (default) (with 3 on the PATH)

  • install 2
  • replace version 3 with 2 in PATH

Dirty Install 2 (no-path) (with 3 on PATH)

  • install 2

MonoRepo Scenario:

Clean Install version X (no-path)

  • install version X

MonoRepo

  • prepends X to PATH

@ThomasFWise
Copy link

I would prefer that an additional option be required to downgrade my path. Especially with as many things as may bundle an call to the installer, I really don't want a simple mistake by one author to downgrade everyone. It's really easy to do too:
Have version 3 installed. Tool runs and just checks
Is version 2 installed? No -> Install version 2
Rather than:
Is version 2 or greater installed? Yes -> Do nothing

There could be scenarios where downgrading the version on the path is correct, but I would prefer it require explicit request.

@reillysiemens
Copy link
Member

@ThomasFWise: From the installer script's perspective, it's not aware that the operation is a downgrade or an upgrade. It's installing whatever version is requested. That's part of the

Avoid any SemVer comparison in the install scripts.

goal that @kyle-rader mentioned. Those comparisons are non-trivial, especially on Unix platforms where there isn't built-in JSON support for interacting with the GitHub API.

The installer actually doesn't even avoid installation if version 2 is installed. If you ask for version 2 and version 2 is already installed it blows away any previously existing installation and installs fresh, which it does to enable repairing a broken installation.

If what you're suggesting is that we do something like add a -UpdatePath flag and switch the default behavior of the installer to not update the $PATH at all I could see that. It would make the installation process less "dangerous" globally. However, it would also make the installer less useful for a fresh installation. The default then would be to install this tool and then have to go configure your $PATH manually.

Instead, with a -NoUpdatePath flag, if you know that you will happen to have multiple competing installations on your system and therefore $PATH updates won't be safe you can opt out of them and manually control the $PATH to your liking.

The danger of downgrading oneself in a multi-version should be addressed here by asserting control over the $PATH (which @kyle-rader touched on by mentioning that a monorepo environment could "prepend X to PATH"). A global install could change the user's $PATH, but it won't get rid of an existing installation, so as long as some environment knows which version it wants to point to it will be available. Our install script appends to $PATH, so it will take the least precedence over any per-environment prepending.

The proposal here is probably best summarized as

We want to stop providing the abstraction of latest because it's proven unreliable. We also want to provide a way to opt out of destructive operations (like $PATH changes) if you know those are unsafe in your environment.

There's a line between being an installer script and a package manager. I'd prefer to stay as much towards the installer script side of that balance. The goal is to get the bits to disk (and maybe make them quickly executable), not to manage versions.

@ThomasFWise
Copy link

You say append, but the Monorepo example says prepend. If you are really appending to the path, then I would be ok with that. Prepend has the problems of potentially silently changing what version someone is using though, so I would be hesitant of that.

I'm not sure you can really say there is a monorepo/non-monorepo divide though. The PATH is not separate, so if I'm working on a project that uses this public tool in a non-monorepo configuration it has the potential to mess up the PATH for my other projects, or even my monorepo configured project.

@reillysiemens
Copy link
Member

The monorepo/non-monorepo divide is not a great way to talk about this, I agree. I was really just trying to describe a scenario where multiple versions of this tool are installed alongside one another. You're 💯% correct that there's only one $PATH.

Both append and prepend are correct in this context, @ThomasFWise. The installation script will only ever append. Because we append, any users managing their own environment are free to prepend, secure in the knowledge that their choice will always take precedence. Any external tooling (initialization scripts, etc.) can temporarily override the user's globally defined $PATH and point to whichever version of azureauth they expect to use in their specific context.

@marcelais
Copy link
Author

One reason we used 'latest' was to avoid having to know the exact install path to AzureAuth. If the install path was predictable, we could get away with not using latest at all but using the version that was installed.

One challenge is that when you ask to install 'v0.3.0' the actual install directory is 'azureauth-v0.3.0-win10-x64' (on my Windows machine) so the installer would have to "know" that full extension. If this is a cross-platform script, then it would have to know all of the various platform flavours.

If the installer were to install to the folder that was named exactly the same as the version that was passed into the script, that would make predicting the path easier and also allow the script to more reliably determine if AzureAuth is already installed before calling the installer.

A similar option would be to have the install path configuration option be the exact path that we install to instead of a "root" directory that you create a versioned directory underneath. [Essentially, letting the calling script manage the version directory structure.]

@kyle-rader
Copy link
Contributor

kyle-rader commented Jun 14, 2022

@ThomasFWise are you satisfied by Tucker's last reply? I think we acknowledge the risks, that a user could install a new version, it updates their PATH, but in our primary monorepo scenario so far, our environment is not broken by this since it does/will prepend a specific version.

@reillysiemens
Copy link
Member

@marcelais: Your suggestion is completely reasonable (and preferable). In fact, when we started out, we used that approach and just named the directory according to its version. Unfortunately, in #49 we undid that as a hotfix for a bug that later turned out to be the result of file lock timing with DLLs for a running azureauth instance (see #99, #100, #102). Now that we taskkill any running azureauth instance before install we should hit that with much less frequency and it's likely safe to revert to a $install_root + $version directory scheme. That would allow us to get rid of latest (which has always been something of a lie) and still have a platform-agnostic install path.

Unfortunately, it's a backwards incompatible change. However, because we've only published 4 versions of azureauth so far we could modify the install script to just do an equality check on those 4 versions and retain the previous behavior (extract to azureauth-${version}-${runtime} and create/update latest). After enough time has elapsed (maybe 6 months or so) we can think about removing the extra logic from the installer.

So, if we accept that, the TL;DR on the proposed changes is:

  1. Update the install script to stop creating a latest directory (unless one of the 4 existing versions is requested).
  2. Update the install script to use platform-agnostic path (unless one of the 4 existing versions is requested).
  3. Add a -NoUpdatePath flag to help avoid modifications to the global $PATH.
  4. Encourage/require that multi-version environments pin the versions they're installing and prepend to the $PATH for any temporary overrides to a global installation.

How does that all sound, @kyle-rader, @marcelais, @ThomasFWise? 🤔

@marcelais
Copy link
Author

Yep, that sounds good.

@ThomasFWise
Copy link

That sounds reasonable.

@kyle-rader
Copy link
Contributor

Closed with #120 - no more latest directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants