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

When using choco push, provide ability to use alias #63

Open
gep13 opened this issue Feb 4, 2015 · 13 comments
Open

When using choco push, provide ability to use alias #63

gep13 opened this issue Feb 4, 2015 · 13 comments

Comments

@gep13
Copy link
Member

gep13 commented Feb 4, 2015

Use Case:

As a user, I want to be able to use the alias name for a source when using choco push

i.e. choco push --source chocolatey is the same as choco push --source http://chocolatey.org/

@gep13 gep13 added this to the 0.9.10 milestone Feb 4, 2015
@ferventcoder ferventcoder modified the milestones: 0.9.10, 0.9.10.x May 20, 2015
@ferventcoder
Copy link
Member

Related to #356

@dragon788
Copy link
Contributor

Definitely would love to see this one, especially since using the -s"http://somelongurl" is potentially error prone, since adding a trailing slash (or not) seems to cause it to not match if it isn't exactly the same as seen in the sources list even if it's the same url. Mostly biting me since the push url doesn't seem to want the /chocolatey or /nuget trailing text.

DamianMaslanka5 added a commit to DamianMaslanka5/choco that referenced this issue Feb 23, 2018
If multiple sources are passed throw exception.
DamianMaslanka5 added a commit to DamianMaslanka5/choco that referenced this issue Feb 26, 2018
If multiple sources are passed throw exception.
@BaconTentacles
Copy link

To be honest, I am a little surprised this wasn't base functionality all along. Given that choco behaves similarly to the nuget CLI, I would have expected "choco push -source MySourceName" to Just Work.

I see this issue is also rather ancient. Are there any plans to ever implement this?

@gep13 gep13 linked a pull request Oct 15, 2021 that will close this issue
TheCakeIsNaOH pushed a commit to TheCakeIsNaOH/choco that referenced this issue Jan 17, 2022
If multiple sources are passed throw exception.
TheCakeIsNaOH pushed a commit to TheCakeIsNaOH/choco that referenced this issue Mar 9, 2022
If multiple sources are passed throw exception.
TheCakeIsNaOH pushed a commit to TheCakeIsNaOH/choco that referenced this issue Mar 11, 2022
If multiple sources are passed throw exception.
TheCakeIsNaOH pushed a commit to TheCakeIsNaOH/choco that referenced this issue Mar 21, 2022
If multiple sources are passed throw exception.
@pauby pauby modified the milestones: 0.11.x, 1.1.0 Mar 24, 2022
gep13 pushed a commit to DamianMaslanka5/choco that referenced this issue Mar 25, 2022
If multiple sources are passed throw exception.
@gep13 gep13 modified the milestones: 1.1.0, 1.x Mar 25, 2022
gep13 pushed a commit to DamianMaslanka5/choco that referenced this issue Mar 25, 2022
If multiple sources are passed throw exception.
TheCakeIsNaOH pushed a commit to TheCakeIsNaOH/choco that referenced this issue Jun 27, 2022
If multiple sources are passed throw exception.
TheCakeIsNaOH pushed a commit to TheCakeIsNaOH/choco that referenced this issue Jan 13, 2023
If multiple sources are passed throw exception.
TheCakeIsNaOH pushed a commit to TheCakeIsNaOH/choco that referenced this issue Mar 16, 2023
If multiple sources are passed throw exception.
vexx32 pushed a commit to TheCakeIsNaOH/choco that referenced this issue Mar 17, 2023
If multiple sources are passed throw exception.
@gep13
Copy link
Member Author

gep13 commented Mar 21, 2023

@vexx32 can you confirm whether this issue has been implemented or not? Based on a PR that you have merged, there is a commit that is tagged against this issue. If it has been addressed, then we should get this added to the milestone.

@vexx32
Copy link
Member

vexx32 commented Mar 21, 2023

@gep13 yes, this was pulled in alongside other changes to choco push.

@gep13
Copy link
Member Author

gep13 commented Mar 21, 2023

Ok, I will get the milestone updated to reflect that this was pulled in.

@gep13 gep13 modified the milestones: 1.x, 2.0.0 Mar 21, 2023
@gep13 gep13 added 4 - Done and removed 3 - Review labels Mar 21, 2023
@gep13
Copy link
Member Author

gep13 commented Apr 19, 2023

While it was originally believed that this issue was solved, it turns out that it wasn't. Let me try to explain...

In order to push to a source, you need to have an API Key defined for that source. Within the chocolatey.config file, you will then have an entry that looks like this:

  <apiKeys>
    <apiKeys source="https://push.chocolatey.org" key="not_a_real_key />
  </apiKeys>

Similarly, when accessing a source for install/upgrade/download of packages, you may need to require authentication, and as such, you might have an entry in the chocolatey.config file, it may look something like this:

<source id="chocolatey" value="https://community.chocolatey.org/api/v2/" disabled="false" bypassProxy="false" selfService="false" adminOnly="false" priority="0" />

In this case, authentication isn't required, but both of these entries (i.e. the apikey and the source) are pointing essentially at the same place, i.e. the Chocolatey Community Repository. However, notice that the URL's are different for each. There is no guarantee that the URL used to consume packages is the same one as the URL used to push packages. As such, we can't assume that the URL used when you use a named source i.e.

choco install packageA --source chocolatey

would be the same as the URL needed to do a push, i.e.

choco push packageA --source chocolatey

The fix that was put in place for this issue was that from the list of sources that are configured, find the URL that matches the alias that was used (assuming that there is one). If found, then go and look for an apikey entry with that URL, and if found, use the API Key for that entry during the push. Since this approach "might" work, but also that is "might not" work, it is deemed as not appropriate to ship at the minute, and the change that was made is planned to be reverted.

The correct fix for this issue going forward would be to add an additional XML attribute to the apikey element, so that it would look something like this:

  <apiKeys>
    <apiKeys name=chocolatey source="https://push.chocolatey.org" key="not_a_real_key />
  </apiKeys>

That way we could guarantee that the alias used for the choco push command mapped directly to the apikey needed when doing the push, and likewise that the correct URL is used when doing choco install/upgrade etc.

Now naming is hard. Whether this new attribute is called name, or id, or key, is up for debate, as we have all of them in use within the chocolatey.config file.

@gep13
Copy link
Member Author

gep13 commented Apr 19, 2023

Just to follow up on this as well, we are going to remove this from the 2.0.0 milestone, as this isn't something we are going to tackle before release.

@gep13 gep13 removed this from the 2.0.0 milestone Apr 19, 2023
@vexx32
Copy link
Member

vexx32 commented Apr 20, 2023

The correct fix for this issue going forward would be to add an additional XML attribute to the apikey element (...)

Looking at this it kinda feels like this is a bit of an odd approach to me. This would potentially let users define multiple different API keys for the same source URL depending on how we do the implementation. Which is maybe a good thing? Not sure.

Adding a name to the apiKey itself, which would then be (imo confusingly) mapped to the --source parameter for choco push command (unless you're thinking differently here, but additional/different params weren't mentioned in your comment)... seems like we're muddying what the --source param is used for. Cognitive load and confusion goes up, I think.

My gut feel is that it would make more sense to just have the source itself support a pushUrl as well as the default url so that those two things can be kept in the source information, and then no changes are necessarily needed in the apiKey (although perhaps it could refer to the source via name rather than direct url?).

@gep13
Copy link
Member Author

gep13 commented Apr 20, 2023

This is not a bad idea, and something that I have considered as a solution, but I wasn't convinced by it. Let me try to explain...

If we add something like a pushUrl to the source element, that would mean that we would have the original source URL (for consumption of packages) along with (if required) the username/password or certificate/password. Then, on top of this we would have the pushUrl but not the api key that is needed to use it. For that, you would need to go off to the apikey section to find it.

While I agree that the disconnect in the usage of --source is confusing, but also I think the disconnect, or split, between the authentication between consumption and pushing of packages, also adding to confusion.

This isn't something that we are solving just now, but it is something that we need to come back to, and decide on how we want to proceed.

@vexx32
Copy link
Member

vexx32 commented Apr 20, 2023

Yeah, that's fair. Might be worth discussing replacing --source for push as well, it's really more a --destination anyway 😂

vexx32 added a commit to vexx32/choco that referenced this issue Apr 20, 2023
We have decided that the current implementation doesn't work as we would
want for some common use cases, and so this commit removes the changes.

Tests have been left behind and flagged as ignored, so we can make use
of them when a design is agreed on and the functionality is
reimplemented.
vexx32 added a commit to vexx32/choco that referenced this issue Apr 20, 2023
We have decided that the current implementation doesn't work as we would
want for some common use cases, and so this commit removes the changes.

Tests have been left behind and flagged as ignored, so we can make use
of them when a design is agreed on and the functionality is
reimplemented.
gep13 pushed a commit to vexx32/choco that referenced this issue Apr 21, 2023
We have decided that the current implementation doesn't work as we would
want for some common use cases, and so this commit removes the changes.

Tests have been left behind and flagged as ignored, so we can make use
of them when a design is agreed on and the functionality is
reimplemented.
gep13 pushed a commit to vexx32/choco that referenced this issue Apr 21, 2023
We have decided that the current implementation doesn't work as we would
want for some common use cases, and so this commit removes the changes.

Tests have been left behind and flagged as ignored, so we can make use
of them when a design is agreed on and the functionality is
reimplemented.
gep13 pushed a commit to vexx32/choco that referenced this issue Apr 21, 2023
We have decided that the current implementation doesn't work as we would
want for some common use cases, and so this commit removes the changes.

Tests have been left behind and flagged as ignored, so we can make use
of them when a design is agreed on and the functionality is
reimplemented.
gep13 added a commit that referenced this issue Apr 21, 2023
(#63) Revert change for `push --source` /w a source name/alias
@AlexNecakov
Copy link

Is there a reason to keep the apiKey and Source elements separate in chocolatey.config at all? If apiKey was just a field in a Source then it wouldn't need its own ID to avoid having to use the URL when pushing or adding the key.
Also agreed that the source element is confusingly named when discussing pushing packages. Maybe feed would be more fitting?

@TheCakeIsNaOH
Copy link
Member

Is there a reason to keep the apiKey and Source elements separate in chocolatey.config at all?

Yes. For example with the Community Repository, there is a separate url that is used for pushes, as compared to the url used for install/upgrade/search. And for users with something like Sonatype Nexus setup, they may have one or more hosted feeds that they push packages to, while only installing/upgrading/searching from a combined group feed.

So, at least to me, it makes sense to keep the push urls separate from the package consumption urls.

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

Successfully merging a pull request may close this issue.

8 participants