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

Non-private registries cause schema issue #3600

Closed
rjmholt opened this issue Apr 29, 2021 · 11 comments
Closed

Non-private registries cause schema issue #3600

rjmholt opened this issue Apr 29, 2021 · 11 comments
Assignees
Labels
T: bug 🐞 Something isn't working

Comments

@rjmholt
Copy link

rjmholt commented Apr 29, 2021

Package ecosystem

NuGet

Language version

Manifest location and content prior to update

dependabot.yml content

Updated dependency

What you expected to see, versus what you actually saw

Native package manager behavior

Images of the diff or a link to the PR, issue or logs

🕹 Bonus points: Smallest manifest that reproduces the issue

From PowerShell/PSScriptAnalyzer#1664.

Config file:

version: 2
registries:
  powershell-gallery:
    type: nuget-feed
    url: https://www.powershellgallery.com/api/v2/
  nuget-org:
    type: nuget-feed
    url: https://api.nuget.org/v3/index.json

updates:
- package-ecosystem: nuget
  directory: "/"
  schedule:
    interval: daily
    time: "13:00"
    timezone: America/Los_Angeles
  open-pull-requests-limit: 10
  registries:
  - powershell-gallery
  - nuget-org

You can find the current file here.

The configuration file is reporting a schema issue:

image

The configuration file I wrote is based on dependabot's own generated configuration in PowerShell/PSScriptAnalyzer#1668 (which we rewrote because we thought GitHub might have special logic to detect dependabot commits to perform a configuration hook).

I can't find any further information on the schema or what it expects and the documentation doesn't really cover this case (it only talks about private registries that have tokens or passwords).

I also saw the same error message with this config here.

  1. Are non-authenticated registry entries supported?
  2. Is there a schema I can refer to to help understand what's going on here (the current error message didn't help)
@rjmholt rjmholt added the T: bug 🐞 Something isn't working label Apr 29, 2021
@rjmholt
Copy link
Author

rjmholt commented Apr 29, 2021

Original dependabot commit which also had issues: PowerShell/PSScriptAnalyzer@9a4a3cd

@xlgmokha xlgmokha self-assigned this Apr 29, 2021
@asciimike
Copy link
Contributor

asciimike commented Apr 29, 2021

@rjmholt, @xlgmokha is going to dive in a little deeper, but my first quick look is that it's missing a username/password or token (docs).

I assume if you add the right auth mechanism to the registry, it will work fine.

@xlgmokha
Copy link
Contributor

xlgmokha commented Apr 29, 2021

Hey @rjmholt! Thanks for submitting this issue.

Is there a schema I can refer to to help understand what's going on here (the current error message didn't help)

At the moment it looks like we haven't published the v2 schema publicly. I'll see what I can do about that.

Hold tight and I'll see what I can dig up. Thanks for your patience.

@xlgmokha
Copy link
Contributor

Here's what I found... When we validate nuget-feed registries we check to see that they have at least a token or username/password pair. Here's a snippet of the JSON schema equivalent validation.

    "nuget-feed": {
      "anyOf": [
        {
          "type": "object",
          "required": [
            "type",
            "url",
            "token"
          ],
          "additionalProperties": false,
          "properties": {
            "type": {
              "type": "string",
              "enum": [
                "nuget-feed"
              ]
            },
            "url": {
              "type": "string"
            },
            "token": {
              "type": "string"
            }
          }
        },
        {
          "type": "object",
          "required": [
            "type",
            "url",
            "username",
            "password"
          ],
          "additionalProperties": false,
          "properties": {
            "type": {
              "type": "string",
              "enum": [
                "nuget-feed"
              ]
            },
            "url": {
              "type": "string"
            },
            "username": {
              "type": "string"
            },
            "password": {
              "type": "string"
            }
          }
        }
      ]
    },

It looks like both api.nuget.org and www.powershellgallery.com are public registries that do not require an Authorization header in order to download packages. Is that correct?

@xlgmokha
Copy link
Contributor

Hey @rjmholt it looks like you already beat me to my next suggestion PowerShell/PSScriptAnalyzer#1671.

Please let me know if that works a little bit better for you.

@rjmholt
Copy link
Author

rjmholt commented Apr 29, 2021

Ah, thanks for the help!

It looks like both api.nuget.org and www.powershellgallery.com are public registries that do not require an Authorization header in order to download packages. Is that correct?

Yes that's correct!

but my first quick look is that it's missing a username/password or token (docs).

I didn't manage to find the section of the docs you linked, but instead ended up here.

That section, combined with the original config that dependabot itself submitted, didn't give me the impression that we needed to provide any auth entries (eventually I read between the lines that the word "private" kept appearing around registries and that the only examples included auth entries).

Perhaps nobody else has/will hit this issue, but some things that might have helped me in this case:

  • The schema validator telling me specifically what was wrong (i.e. what entry it was expecting or what entry wasn't allowed)
  • A link to the schema from the validator so I could understand the expectation
  • A description of the structure of registries entries in the docs, namely what keys are available and which are required
  • A call-out in the docs that registries is only for private, authenticated registries and an auth configuration must be provided

Please let me know if that works a little bit better for you.

After removing the explicit registry entires, it all seems to be working now.

Really appreciate you following up so quickly and finding that schema — that's exactly what I was looking for to get me unblocked!

@rjmholt
Copy link
Author

rjmholt commented Apr 29, 2021

Oh, also, is there a way to configure the magical dependabot validation CI task to check any edits to the config file, rather than just when dependabot first adds it? That would be really useful to see if we're about to commit a bad config file.

@xlgmokha
Copy link
Contributor

xlgmokha commented Apr 29, 2021

Dependabot-Preview shouldn't be opening up PR's that include registry config without valid credentials. I'll follow up on that to make sure.

The schema validator telling me specifically what was wrong (i.e. what entry it was expecting or what entry wasn't allowed)

I agree. This would have helped me debug this issue as well.

A link to the schema from the validator so I could understand the expectation

I'll see if we can publish a schema in some form in the docs. @asciimike is this something that we can do?

A description of the structure of registries entries in the docs, namely what keys are available and which are required

This sounds do-able.

A call-out in the docs that registries is only for private, authenticated registries and an auth configuration must be provided

I agree. I captured the bullet points and dropped them into the internal issue tracker for us to prioritize. Thanks for taking the time to write that up.

Oh, also, is there a way to configure the magical dependabot validation CI task to check any edits to the config file, rather than just when dependabot first adds it? That would be really useful to see if we're about to commit a bad config file.

I wish. At least not that I know of yet but you do have the attention of the right people to make that happen. 😉

I personally would love to be able to type gh lint .github/dependabot.yml (Shameless plug for gh cli).

Feel free to re-open this issue if you run into any other related issues. Happy hacking!

@xlgmokha
Copy link
Contributor

@rjmholt you were 💯 right that Dependabot-Preview was opening up migration PR's with missing credentials when the registry was configured as a public registry. I'm working on a patch now. Thank you, thank you, thank you!

@rjmholt
Copy link
Author

rjmholt commented Apr 29, 2021

Really appreciate all your help and friendliness — you've made my day 🙂

@jurre
Copy link
Member

jurre commented Apr 30, 2021

Oh, also, is there a way to configure the magical dependabot validation CI task to check any edits to the config file, rather than just when dependabot first adds it? That would be really useful to see if we're about to commit a bad config file.

It should actually run on any edit, but it appears the check is currently on ran on the default branch. That's unfortunate because it's most useful when it's ran on a pull request changing the file! I'll look into fixing that, but I might not be able to prioritize it today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants