V4#3807
Conversation
f8066e1 to
d28fd70
Compare
8b3d172 to
7686fe7
Compare
638c3b4 to
a52917e
Compare
Remove header times, ANSI timestamps only and always
|
On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/1625 |
There was a problem hiding this comment.
I found a few blocking issues in the CLI v3 and OpenTelemetry changes. The PR is L3, and the Buildkite agent check was still pending while I reviewed.
Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 1625, then answer my questions about the findings.
|
On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/1656 |
Previous concerns have been addressed.
There was a problem hiding this comment.
Previous buildsworth blockers are resolved: OTel provider init now follows opentelemetry-tracing / TelemetryServiceName, GCP metadata tags are a bool flag, and the cloud tag wait timeouts are time.Duration. I found one new blocking SSH host-key fallback issue, plus a config-template cleanup.
Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 1656, then answer my questions about the findings.
Upgrade urfave/cli to v3
|
On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/1703 |
There was a problem hiding this comment.
Previous blocking SSH fallback concern is resolved by failing closed when the version can't be determined. The packaged config-template cleanup is still partly open, and I found one new blocking SSH setup regression. The PR remains L3 because it changes agent checkout/security/config behaviour; the Buildkite agent check was still pending while I reviewed.
Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 1703, then answer my questions about the findings.
|
On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/1726 |
|
On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/1728 |
|
On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/1731 |
There was a problem hiding this comment.
The latest non-SSH setup failure and packaged config-template cleanup are addressed. The follow-up reintroduces the SSH-version fallback blocker from the prior round, so I'm requesting changes; the Buildkite agent check is currently failing.
Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 1728, then answer my questions about the findings.
There was a problem hiding this comment.
The active buildsworth SSH host-key blocker from the previous pass is still unresolved; I didn't find anything else beyond that on this pass. The PR remains L3 because it changes agent checkout/security/config behavior, and the Buildkite agent check is currently failing.
Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 1731, then answer my questions about the findings.
…ecking=accept-new Amp-Thread-ID: https://ampcode.com/threads/T-019e67ed-1ae1-712b-a9d5-dca820e080a1 Co-authored-by: Amp <amp@ampcode.com>
A-1118: Replace addRepositoryHostToSSHKnownHosts with StrictHostKeyChecking=accept-new
chore: replace unlabelled jobStarted and jobEnded metrics
Replace dogstatsd metrics with opentelemetry metrics (take 2)
|
On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/1743 |
There was a problem hiding this comment.
Previous buildsworth concerns are addressed in the current branch: the SSH host-key fallback now avoids StrictHostKeyChecking=no when the SSH version cannot be determined, and the older packaged config-template cleanup is also resolved. I don't have new inline findings from this pass; I'm leaving this as a comment rather than an approval because the PR is L3 (agent checkout/security/config behavior changes), and the Buildkite agent check is still pending.
Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 1743, then answer my questions about the findings.
Description
Make v4 happen.
The plan is:
VERSIONto4.0.0to do a stable v4 release 🎉Context
It's about time.
Fixes #1391
Fixes #1594
Fixes #1623
Fixes #1646
Closes #1593
Changes
allow-artifact-path-traversal: removed, the insecure behaviour is no longer supportednormalised-upload-paths: is now default behaviouroverride-zero-exit-on-cancel: is now default behaviourresolve-commit-after-checkout: is now default behaviourpropagate-agent-config-vars: is now default behaviourdescending-spawn-priority: removed, with the--spawn-with-priorityflag now taking a string value (one ofstatic,ascending, ordescending)trace-context-encodingkubernetes-log-collection-grace-periodno-automatic-ssh-fingerprint-verification(useno-ssh-keyscaninstead)meta-data(usetagsinstead)meta-data-ec2(usetags-from-ec2-meta-datainstead)meta-data-ec2-tags(usetags-from-ec2-tagsinstead)meta-data-gcp(usetags-from-gcp-meta-datainstead)tags-from-ec2(usetags-from-ec2-meta-datainstead)tags-from-gcp(usetags-from-gcp-meta-datainstead)disconnect-after-job-timeout(usedisconnect-after-idle-timeoutinstead )follow-symlinks(useglob-resolve-follow-symlinksinstead)buildkite-agent meta-data getcancel-grace-periodandsignal-grace-period-secondsflags withcancel-signal-timeoutandcancel-cleanup-timeout, and adjust the timeouts (10s signal timeout and 5s cleanup timeout)reject-secretsis replaced withallow-secrets)Testing
go test ./...). Buildkite employees may check this if the pipeline has run automatically.go tool gofumpt -extra -w .)Disclosures / Credits
jj rebase