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

[bug] conan install --update should fail if package isn't found on remote #11022

Closed
PeteAudinate opened this issue Apr 13, 2022 · 7 comments
Closed

Comments

@PeteAudinate
Copy link

Environment Details (include every applicable attribute)

  • Operating System+version: Windows 11
  • Compiler+version: N/A
  • Conan version: 1.44.1
  • Python version: 3.9.1

Steps to reproduce (Include if Applicable)

  • Add an Artifactory remote with anonymous usage enabled
  • Enable revisioning
  • Upload some private packages
  • Ensure you are not logged in to the remote
  • Ensure you have a revision of the packages in your cache (but not necessarily the latest revision)
  • Try to update these packages to the latest revision:
    $ conan.exe install . --update --profile myprofile --profile:build myprofile
    

Logs (Executed commands with output) (Include/Attach if Applicable)

Configuration (profile_host):
...

Configuration (profile_build):
...

asio/1.18.2: WARN: Can't update, no package in remote
boost/1.78.0: WARN: Can't update, no package in remote
cli11/2.0.0: WARN: Can't update, no package in remote
...

conanfile.py: Installing package
Requirements
    asio/1.18.2 from 'myremote' - Not in remote
    boost/1.78.0 from 'myremote' - Not in remote
    cli11/2.0.0 from 'myremote' - Not in remote
...

Packages
    asio/1.18.2:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Cache
    boost/1.78.0:e4927933ff57d4bc8c5183cd93ad1e12d690d2d4 - Cache
    cli11/2.0.0:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Cache
...

Installing (downloading, building) binaries...
asio/1.18.2: Already installed!
boost/1.78.0: Already installed!
boost/1.78.0: Disabled magic autolinking (smart and magic decisions)
cli11/2.0.0: Already installed!
...

conanfile.py: Generator 'CMakeToolchain' calling 'generate()'
conanfile.py: Generator txt created conanbuildinfo.txt
conanfile.py: Generator 'CMakeDeps' calling 'generate()'
conanfile.py: Aggregating env generators
conanfile.py: Generated conaninfo.txt
conanfile.py: Generated graphinfo

Expected Behaviour

conan install --update command should fail if the package isn't found on the remote, as there may be a more recent revision available.

Actual Behaviour

Command succeeds.

Comments

The response to #10918 (comment) concluded that conan install --update should hard-fail if the remote can't be reached, since otherwise "this behavior can easily hide network failures, configuration errors, and other problems, silently producing unexpected results".

In this case, if the user unexpectedly becomes unauthenticated (either explicitly or due to a credential timeout), Artifactory returns a 404 for all package operations. That causes conan install --update to "silently produce unexpected results" and the user needs to look carefully at the logs to notice that there was an authentication error.

See #4930 and #8127 for more details on how Artifactory behaves when the user is not properly authenticated.

@memsharded
Copy link
Member

Hi @PeteAudinate

The response to #10918 (comment) concluded that conan install --update should hard-fail if the remote can't be reached, since otherwise "this behavior can easily hide network failures, configuration errors, and other problems, silently producing unexpected results".

In this case, if the user unexpectedly becomes unauthenticated (either explicitly or due to a credential timeout), Artifactory returns a 404 for all package operations. That causes conan install --update to "silently produce unexpected results" and the user needs to look carefully at the logs to notice that there was an authentication error.

Failing a --update because some package doesn't exist in a remote is unfeasible. Many times, such packages will not exist in the remotes, when developing new packages locally, or simply because the remote it came from is temporarily disabled. There can be multiple remotes too, so it is not enough that it doesn't exist in 1 remote to fail. So Conan will not fail if it requests its defined and enabled remotes if there are updates and the servers respond with 404s, because those packages doesn't exist in the servers. The UX for that will be quite bad for a large number of users, even make the --update impractical.

Now, if the security configuration makes a server return a 404 even when the package exists, but the user doesn't have enough privileges to see it, is a different story. Good security practices says that in general it is better to return 404 rather than return other thing that will provide information of the existence of a given resource. There is little we can do here.

So, as a result, this doesn't look like a bug, but the expected behavior, I don't think we want to change this behavior. If servers require auth for "read" permissions over packages, and the --update operation is critical, then user (sounds the case would typically be CI) should verify first it is actually authenticated against the server, maybe using conan user command, for example

@PeteAudinate
Copy link
Author

Thanks @memsharded for your thoughtful reply :-).

The UX for that will be quite bad for a large number of users, even make the --update impractical.

Understood. I think there are valid use cases for both behaviours though. The current behaviour of "assume 404 is fine" isn't very useful for the simplest enterprise use case of a single Artifactory remote hosting all packages. I see that there are more complex configurations where you'd want to allow it to fail silently, but I'd have thought "tell me if it fails" would be a pretty common expectation.

So, as a result, this doesn't look like a bug, but the expected behavior, I don't think we want to change this behavior.

That sounds fair. However it would still be useful to have some way of knowing that the install --update might have failed due to a 404 response from the server. We could do that ourselves by manually parsing the Conan output for "WARN: Can't update, no package in remote", but that's a bit hacky and fragile.

(sounds the case would typically be CI)

No, I think this is primarily an issue for end-users using a conanfile.txt to maintain the dependencies of a project. Currently we use conan install --update to ensure we have the latest dependency revisions locally before running a build. It would be a pain to have to run conan user and manually enter a password/API key each time we do this.

The CI case is more straightforward, since you can just explicitly log in before running conan install --update (assuming you want to share the cache between builds). But it's not practical to require an end-user to do this every time just in case they've become unauthenticated.

If servers require auth for "read" permissions over packages, and the --update operation is critical, then user (sounds the case would typically be CI) should verify first it is actually authenticated against the server, maybe using conan user command, for example

This sounds promising. Is there a way to run a conan user command to determine whether the user is currently authenticated? I couldn't find a way, but perhaps I missed something.

@memsharded
Copy link
Member

This sounds promising. Is there a way to run a conan user command to determine whether the user is currently authenticated? I couldn't find a way, but perhaps I missed something.

You are right, there is no way that a stored token could be validated without requiring entering credentials again.
I'll add this for discussion (for 2.0 scope, new features are very unlikely to happen now in 1.X), as I think this use case would make sense.

@czoido
Copy link
Contributor

czoido commented Feb 21, 2023

Closed by #13180

@czoido czoido closed this as completed Feb 21, 2023
@memsharded
Copy link
Member

#13180 is proposing a new command conan remote auth that can enforce real authentication in the different defined remotes, based on user input, or environment variables, or use the cached tokens to guarantee an active auth is valid.

This is the best that the client can do, it is a responsibility of the server to return 404 not found for queries if not authenticated, without raising an auth challenge if they activate anonymous usage. And it doesn't make sense that the client is continuously authenticating in the servers just in case, so the best is a explicit user command that does this task.

@PeteAudinate
Copy link
Author

Sounds perfect, thanks. We've worked around this for 1.x by searching for a package known to always be on the remote and raising a "not authenticated" error message if the package can't be found. But an explicit command is much better 👍 .

@PeteAudinate
Copy link
Author

@memsharded, @czoido, the conan remote auth command doesn't seem to resolve this. It looks like it doesn't differentiate bewteen a valid authentication and an expired token.

No problems here:

$ conan remote auth eng-conan-local
eng-conan-local:
    user: me@mydomain.com

But I can't find any packages:

$ conan list protobuf -r=eng-conan-local
eng-conan-local
  ERROR: Recipe 'protobuf' not found

Try re-authenticating:

$ conan remote login eng-conan-local me@mydomain.com
Please enter a password for "me@mydomain.com" account:
Changed user of remote 'eng-conan-local' from 'me@mydomain.com' (anonymous) to 'me@mydomain.com' (authenticated)

Now I can see packages:

$ conan list protobuf -r=eng-conan-local
eng-conan-local
  protobuf
    protobuf/3.19.1
    protobuf/3.19.6
    protobuf/3.20.0

But conan remote auth returns the same result:

$ conan remote auth eng-conan-local
eng-conan-local:
    user: me@mydomain.com

Is this working as expected? I can't use conan remote auth unless it's able to tell me whether I'm properly authenticated with a remote.

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

No branches or pull requests

3 participants