arcxp / datadog-service-catalog-metadata-provider

This repository houses the Datadog Service Catalog Metadata Provider. With this tool you can use GitHub Actions to provide Datadog with the metadata for your service. For more information on what the Datadog Service Catalog is: https://www.datadoghq.com/product/service-catalog/
MIT License
19 stars 6 forks source link

crash: inputsToRegistryDocument returning non-promise #108

Closed hermanbanken closed 3 months ago

hermanbanken commented 4 months ago

Describe the bug Failure in job, due to non-Promise returned from inputsToRegistryDocument().

To Reproduce use yaml below, and version 4d76927255c82b257b15167485e5f1ecaa96b327 (latest)

Expected behavior Working upload

Screenshots

Screenshot 2024-05-06 at 12 59 18

Workflow definition (PLEASE REDACT ANY SENSITIVE INFORMATION):

on:
  push:

jobs:
  deploy:
    steps:
      - name: Update application in datadog service catalog 
        uses: arcxp/datadog-service-catalog-metadata-provider@4d76927255c82b257b15167485e5f1ecaa96b327 # v2.3.1
        with:
          schema-version: v2.1
          datadog-hostname: api.datadoghq.eu
          datadog-key: ${{ secrets.DD_EU_API_KEY }}
          datadog-app-key: ${{ secrets.DD_EU_APP_KEY }}
          service-name: my-app
          description: my-app is a nice application
          lifecycle: production
          slack-support-channel: https://slack.com/archives/CCCCCCCC
          team: my-team
          email: my-team@my-org.com
          links: |
            - name: ${{ github.repository }}
              url: https://github.com/my-org/my-app
              type: repo
              provider: github
            - name: production api url
              url: https://myapp.com
              type: url    

Additional context Add any other context about the problem here.

manchicken commented 4 months ago

Thanks for your submission! I'll look into this.

mewc commented 4 months ago
/home/runner/work/_actions/arcxp/datadog-service-catalog-metadata-provider/v2.3.1/dist/index.cjs:35854
inputsToRegistryDocument().then((configs) => {
                           ^

TypeError: inputsToRegistryDocument(...).then is not a function
    at Object.<anonymous> (/home/runner/work/_actions/arcxp/datadog-service-catalog-metadata-provider/v2.3.1/dist/index.cjs:35854:[28](https://github.com/kanopicover/kanopi-mono/actions/runs/9012036520/job/24760861234#step:2:29))
    at Module._compile (node:internal/modules/cjs/loader:1241:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1[29](https://github.com/kanopicover/kanopi-mono/actions/runs/9012036520/job/24760861234#step:2:30)5:10)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Module._load (node:internal/modules/cjs/loader:938:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
    at node:internal/main/run_main_module:23:47

Node.js v20.8.1

Getting the same thing.


with:
    schema-version: v2
    datadog-hostname: api.datadoghq.com
    datadog-key: ***
    datadog-app-key: ***
    service-name: server
    repos: - name: kanopi-mono
    url: https://github.com/dddddd
    provider: github

    docs: - name: outage-runbook
    url: https://www.notion.so/mmm
    provider: github

    contacts: - name: Kanopi Tech
    type: email
    contact: tech@mmmmmcascascasca.com

    tags: - isprod:false
  - stage:qa
  - lang:py
manchicken commented 4 months ago

@mewc Thanks! I appreciate the additional context.

manchicken commented 4 months ago

@mewc, could you please update your comment? I think that in the process of pasting the contents, the formatting got mangled somehow. It does not parse as valid YAML.

@hermanbanken, could you please test on the @v2 tag? I think there was a race condition with the build on 2.3.1 and despite that being the latest released version number, the dist/index.cjs artifact is very different between v2 and v2.3.1. Whenever the dist files are finished being set up, it updates the v2 tag, so my recommendation would be to always just use the top-level tag of v2 (or here soon when I finish it, v3).

Let me know if there's still a problem on the v2 tag and if you can still replicate the issue then I'll dig deeper. The YAML you gave me works just fine on the v2 tag in my test environment.

hermanbanken commented 4 months ago

🤔 Not sure about that policy. What are you saying exactly? If 2.3.1 is the latest tag, it should be identical to any updated v2, right? Is there a difference?

We're using Renovate (like millions of others probably), and it automatically proposes to update to the latest release published, which is v2.3.1, not v2. (It does not look at updated tags).

manchicken commented 4 months ago

It's pretty common in public GitHub Actions to use the major release. The major actions (actions/checkout, actions/setup-node, etc) all follow this pattern as well. What it lets us do it fix functionality without making users change code to take updates.

It creates an extra burden for us of backwards-compatibility, but that's a small price to pay for being able to fix bugs for everybody without having to burn time posting update articles and asking people to update.

This GitHub Action will follow that pattern of always updating the major release tag for user convenience. You're free to use whichever tag or commit SHA you like, I won't tell you to do differently.

That said, I am going to re-build and re-tag v2.3.1 as I can't have versions out there where the build artifact doesn't match the code.

manchicken commented 4 months ago

That release has been re-tagged; please let me know if it's working for you now.

manchicken commented 3 months ago

Any problems with the updated tag?

hermanbanken commented 3 months ago

I'm not saying GitHub or others are doing it wrong. I'll quote you:

That said, I am going to re-build and re-tag v2.3.1 as I can't have versions out there where the build artifact doesn't match the code.

I think there was a race condition with the build on 2.3.1 and despite that being the latest released version number, the dist/index.cjs artifact is very different between v2 and v2.3.1

I was mostly confused by the difference between "both latest versions" v2 and v2.3.1.

I think this race condition is a little scary, and is exactly why we use tagging to SHA's! It would be possible for authors of actions to inject all kinds of exfill/security risks if we'd use "@v2" and it keeps updating automatically. We do review all updates (n.b. yours contained no weird/unsafe code, it just didn't work for us).

I'll test the new tag (3b9379b, replacing 4d76927).

hermanbanken commented 3 months ago

It works now. Thanks!

manchicken commented 3 months ago

Yeah, it was a "race condition" insomuch as I had the build happen in a different step locally than the one which published the release.

I understand the concern which leaves folks pegging specific commits, but please understand that the fact that you're pegging commits was the reason you experienced a problem. Common practice is to tag the major version tag, and those who were following that practice never experienced this issue.

manchicken commented 3 months ago

I was mostly confused by the difference between "both latest versions" v2 and v2.3.1.

Again, this is the most common pattern in Custom GitHub Actions. We're adhering to that pattern in order to make sure we're meeting expectations of users familiar with these more common patterns.

I think this race condition is a little scary, and is exactly why we use tagging to SHA's! It would be possible for authors of actions to inject all kinds of exfill/security risks if we'd use "@v2" and it keeps updating automatically. We do review all updates (n.b. yours contained no weird/unsafe code, it just didn't work for us).

The ironic thing here is that if you had used @v2, you would not have experienced this issue. This action is still pretty new, and folks have only recently started using it. As folks give me feedback I will continue to improve the project as all folks do. I apologize that this is scary for you.

If you're depending on pinning versions to protect yourself from breaking changes and attack, you may wish to find another tool. Since GitHub Actions are pulled from their repository at run time (in all cases, excepting cache), pinning commit SHAs is more likely to cause problems than not as there have been security problems in dependencies that I've fixed in this Action, and when I update dependencies I have no way of telling you why the update took place.

mewc commented 3 months ago

Thank you @manchicken