Verify yaml #4639

bengl · 2024-08-29T15:48:58Z

Ensures that plugins are tested with the same ranges that they support.

github-actions · 2024-08-29T15:49:38Z

Overall package size

Self size: 8.26 MB
Deduped: 94.55 MB
No deduping: 95.07 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.2.2 | 29.27 MB | 29.27 MB | | @datadog/native-appsec | 8.3.0 | 19.37 MB | 19.38 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.6.0 | 2.58 MB | 2.72 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.0.1 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

packages/datadog-instrumentations/src/aerospike.js

pr-commenter · 2024-08-29T15:56:26Z

Benchmarks

Benchmark execution time: 2024-12-13 21:15:16

Comparing candidate commit b5c8eb2 in PR branch bengl/verify-yaml with baseline commit 83c6928 in branch master.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 262 metrics, 3 unstable metrics.

scenario:plugin-graphql-with-depth-and-collapse-on-18

🟩 max_rss_usage [-86.749MB; -85.771MB] or [-9.163%; -9.059%]

Qard · 2024-08-29T19:19:24Z

.github/workflows/plugins.yml

@@ -16,6 +16,10 @@ concurrency:

 jobs:
  aerospike-node-16:
+    strategy:
+      matrix:
+        node-version: [16]


Seems like node-version is not actually used?

Yeah, it's for validation in the script. I'm gonna try to work out a better way of expressing this though.

datadog-datadog-prod-us1 · 2024-08-29T20:26:23Z

.github/workflows/project.yml

+        node-version: [16, 18, 20, 22]
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4


🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

Pin third party actions by hash, or at least by tag for trusted sources

When using a third party action, one needs to provide its GitHub path (owner/project) and can eventually pin it to a git ref (a branch name, a git tag, or a commit hash).

No pinned git ref means the action will use the latest commit of the default branch each time it runs, eventually running newer versions of the code that were not audited by Datadog. Specifying a git tag is better, but since they are not immutable, using a full length hash is recommended to make sure the action content is actually frozen to some reviewed state.

Be careful however, as even pinning an action by hash can be circumvented by attackers still. For instance, if an action relies on a Docker image which is itself not pinned to a digest, it becomes possible to alter its behaviour through the Docker image without actually changing its hash. You can learn more about this kind of attacks in Unpinnable Actions: How Malicious Code Can Sneak into Your GitHub Actions Workflows. Pinning actions by hash is still a good first line of defense against supply chain attacks.

Additionally, pinning by hash or tag means the action won’t benefit from newer version updates if any, including eventual security patches. Make sure to regularly check if newer versions for an action you use are available. For actions coming from a very trustworthy source, it can make sense to use a laxer pinning policy to benefit from updates as soon as possible.

Shouldn't actions always be a trusted org? If we can't trust GitHub then why is our code on GitHub?

Can we just disable this entirely? It just complains about everything, making it effectively useless as it just pollutes PRs with dozens of comments like this constantly everywhere.

codecov · 2024-08-29T20:27:18Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.85%. Comparing base (823cfd4) to head (3fa3454).
Report is 30 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4639      +/-   ##
==========================================
- Coverage   65.05%   64.85%   -0.20%     
==========================================
  Files         304      127     -177     
  Lines       13950     4146    -9804     
==========================================
- Hits         9075     2689    -6386     
+ Misses       4875     1457    -3418

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

datadog-datadog-prod-us1 · 2024-08-30T18:14:41Z

.github/workflows/project.yml

+    steps:
+      - uses: actions/checkout@v4
+      - uses: ./.github/actions/install
+      - uses: actions/setup-node@v3


🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

Pin third party actions by hash, or at least by tag for trusted sources

When using a third party action, one needs to provide its GitHub path (owner/project) and can eventually pin it to a git ref (a branch name, a git tag, or a commit hash).

No pinned git ref means the action will use the latest commit of the default branch each time it runs, eventually running newer versions of the code that were not audited by Datadog. Specifying a git tag is better, but since they are not immutable, using a full length hash is recommended to make sure the action content is actually frozen to some reviewed state.

Be careful however, as even pinning an action by hash can be circumvented by attackers still. For instance, if an action relies on a Docker image which is itself not pinned to a digest, it becomes possible to alter its behaviour through the Docker image without actually changing its hash. You can learn more about this kind of attacks in Unpinnable Actions: How Malicious Code Can Sneak into Your GitHub Actions Workflows. Pinning actions by hash is still a good first line of defense against supply chain attacks.

Additionally, pinning by hash or tag means the action won’t benefit from newer version updates if any, including eventual security patches. Make sure to regularly check if newer versions for an action you use are available. For actions coming from a very trustworthy source, it can make sense to use a laxer pinning policy to benefit from updates as soon as possible.

tlhunter

Seems better than what we have today. That said I'm not sure how a developer can figure out how to fix things based on the resulting error.

rochdev · 2024-09-12T15:40:44Z

.github/workflows/project.yml

@@ -147,3 +147,10 @@ jobs:
      - run: yarn type:test
      - run: yarn type:doc

+  verify-yaml:


I think this (and the name of the script file too) could use a more descriptive name of what it's doing. Something like verify-plugin-versions or anything more descriptive.

bengl · 2024-12-12T22:15:36Z

Note that this now removes support (i.e. no longer instruments) Next.js >=12.0.0 <12.2.0 Since the tests won't run (because next build does not successfully run).

rochdev · 2024-12-13T19:13:25Z

Note that this now removes support (i.e. no longer instruments) Next.js >=12.0.0 <12.2.0 Since the tests won't run (because next build does not successfully run).

Do you know why they won't run? According to Next documentation, Next 12 supports Node.js >=12.

rochdev · 2024-12-13T19:14:22Z

.github/workflows/plugins.yml

        include:
+          - node-version: 14 # yes, out of range, but the only way to test the oldest version


Doesn't that mean that this version isn't supported anymore?

We'd have to take it out of instrumentation as well, not just the GitHub yaml. Meaning we'd never attempt to to instrument those versions. Is that fine?

rochdev · 2024-12-13T19:16:43Z

packages/datadog-instrumentations/src/next.js

  file: 'dist/next-server/server/serve-static.js'
 }, serveStatic => shimmer.wrap(serveStatic, 'serveStatic', wrapServeStatic))

-addHook({ name: 'next', versions: ['>=11.1'], file: 'dist/server/next-server.js' }, nextServer => {
+addHook({ name: 'next', versions: ['>=11.1 <12.0.0', '>=12.2.0'], file: 'dist/server/next-server.js' }, nextServer => {


This is a breaking change.

For the versions removed from the range (12.0.x and 12.1.x), next build fails, so the tests fail. It doesn't appear that we can test these versions (or that we ever could) so the claim of support was always incorrect.

* add script to verify plugin yaml * add github actions job to verify yaml * fix instrumentations * fix up aerospike * better version ranges for aerospike * fix ci script * make it pass hopefully * update license 3rdparty * fix it no longer assuming nodejs versions * fix aerospike * since node version is now ignored, run on only one version of node

bengl requested review from a team as code owners August 29, 2024 15:48

tlhunter reviewed Aug 29, 2024

View reviewed changes

packages/datadog-instrumentations/src/aerospike.js Outdated Show resolved Hide resolved

tlhunter added the semver-patch label Aug 29, 2024

Qard reviewed Aug 29, 2024

View reviewed changes

bengl force-pushed the bengl/verify-yaml branch 2 times, most recently from a9149f6 to 6c1a50f Compare August 29, 2024 20:25

datadog-datadog-prod-us1 bot reviewed Aug 29, 2024

View reviewed changes

bengl force-pushed the bengl/verify-yaml branch from 97d938b to f302e1e Compare August 30, 2024 18:13

datadog-datadog-prod-us1 bot reviewed Aug 30, 2024

View reviewed changes

bengl force-pushed the bengl/verify-yaml branch from 98d2f89 to 1d4c5ed Compare August 30, 2024 19:07

bengl force-pushed the bengl/verify-yaml branch 2 times, most recently from f93c231 to 1bbc567 Compare September 11, 2024 20:47

tlhunter previously approved these changes Sep 11, 2024

View reviewed changes

bengl dismissed tlhunter’s stale review via b6d1e93 September 12, 2024 03:04

rochdev reviewed Sep 12, 2024

View reviewed changes

rochdev previously approved these changes Sep 12, 2024

View reviewed changes

Qard approved these changes Sep 12, 2024

View reviewed changes

bengl force-pushed the bengl/verify-yaml branch from b6d1e93 to c4078f3 Compare December 12, 2024 18:44

bengl enabled auto-merge (squash) December 12, 2024 18:44

bengl dismissed rochdev’s stale review via 2b32645 December 12, 2024 21:37

bengl requested a review from a team as a code owner December 12, 2024 21:37

bengl force-pushed the bengl/verify-yaml branch from b4c28fe to c334ed9 Compare December 12, 2024 22:14

tlhunter previously approved these changes Dec 12, 2024

View reviewed changes

bengl force-pushed the bengl/verify-yaml branch from c334ed9 to 5a4d6a9 Compare December 13, 2024 16:58

add script to verify plugin yaml

5ec2c0f

bengl added 11 commits December 13, 2024 12:33

fix instrumentations

8fff568

fix up aerospike

8fbf028

better version ranges for aerospike

3dfa423

fix ci script

8a86280

make it pass hopefully

0010953

update license 3rdparty

4db06d6

fix it no longer assuming nodejs versions

0cd676f

fix aerospike

a6e20ca

since node version is now ignored, run on only one version of node

7b29bd8

fix next

b406d23

remove 12.0.0 to 12.1.x from next version range

ebcf4ef

bengl dismissed tlhunter’s stale review via 3fa3454 December 13, 2024 17:33

bengl force-pushed the bengl/verify-yaml branch from 5a4d6a9 to 3fa3454 Compare December 13, 2024 17:33

lint

2fcd426

bengl force-pushed the bengl/verify-yaml branch from 3fa3454 to 2fcd426 Compare December 13, 2024 17:41

rochdev requested changes Dec 13, 2024

View reviewed changes

tlhunter previously approved these changes Dec 13, 2024

View reviewed changes

remove explit 12.0 and 12.1 tests for next.js

b5c8eb2

bengl dismissed tlhunter’s stale review via b5c8eb2 December 13, 2024 21:04

remove version of aerospike that we don't actually support

72bf1cc

rochdev approved these changes Dec 13, 2024

View reviewed changes

bengl merged commit baf22d9 into master Dec 13, 2024
294 of 300 checks passed

bengl deleted the bengl/verify-yaml branch December 13, 2024 21:17

rochdev mentioned this pull request Dec 17, 2024

v4.53.0 proposal #5030

Merged

rochdev mentioned this pull request Dec 17, 2024

v5.29.0 proposal #5031

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verify yaml #4639

Verify yaml #4639

bengl commented Aug 29, 2024

github-actions bot commented Aug 29, 2024 •

edited

Loading

pr-commenter bot commented Aug 29, 2024 •

edited

Loading

Qard Aug 29, 2024

bengl Aug 29, 2024

datadog-datadog-prod-us1 bot Aug 29, 2024

tlhunter Sep 11, 2024

rochdev Sep 12, 2024

codecov bot commented Aug 29, 2024 •

edited

Loading

datadog-datadog-prod-us1 bot Aug 30, 2024

tlhunter left a comment

rochdev Sep 12, 2024

bengl commented Dec 12, 2024

rochdev commented Dec 13, 2024

rochdev Dec 13, 2024

bengl Dec 13, 2024

rochdev Dec 13, 2024

bengl Dec 13, 2024

		include:
		- node-version: 14 # yes, out of range, but the only way to test the oldest version

Verify yaml #4639

Verify yaml #4639

Conversation

bengl commented Aug 29, 2024

github-actions bot commented Aug 29, 2024 • edited Loading

Overall package size

pr-commenter bot commented Aug 29, 2024 • edited Loading

Benchmarks

scenario:plugin-graphql-with-depth-and-collapse-on-18

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datadog-datadog-prod-us1 bot Aug 29, 2024

Choose a reason for hiding this comment

🟠 Code Vulnerability

Pin third party actions by hash, or at least by tag for trusted sources

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 29, 2024 • edited Loading

Codecov Report

datadog-datadog-prod-us1 bot Aug 30, 2024

Choose a reason for hiding this comment

🟠 Code Vulnerability

Pin third party actions by hash, or at least by tag for trusted sources

tlhunter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bengl commented Dec 12, 2024

rochdev commented Dec 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 29, 2024 •

edited

Loading

pr-commenter bot commented Aug 29, 2024 •

edited

Loading

codecov bot commented Aug 29, 2024 •

edited

Loading