Add OS Package Repo Tool 2 (OPRT2) by fheinecke · Pull Request #400 · gravitational/shared-workflows

fheinecke · 2025-09-25T20:20:37Z

Summary

This is a first pass at a new revision of the OS Package Repo Tool (OPRT). The primary purpose of this is to public OS packages via Attune, and is intended to replace practically all of our current tooling around OS package publishing.

Goals

As discussed internally, this tool will operate under the following assumptions:

AWS auth is handled outside of program (this is environment specific)
Teleport auth is handled outside of program (this is environment specific)
Configuration is provided outside of program (this is environment specific)
Required binaries (attune CLI, Teleport CLI, others required in the future) will be present in the execution environment

Also discussed internally, this tool is designed to meet the following criteria:

It must be able to run both locally and in CI/CD pipelines
- This affects where packages will be pulled from as well as how Attune authentication is handled
It must run significantly faster than our current process
- This affects what code needs to run concurrently
It must support our disaster recovery (rebuilding the entire prod repo from scratch)
- This affects everything related to supporting a longer runtime, like credential renewal
It must not require running through a Teleport-backed proxy (e.g. tsh proxy app attune)
It must "nicely" support both being ran as a CLI tool with our current CI/CD pipelines, and should support being used as a library for our replacement of GHA (whatever this ends up looking like)

The only component in this PR that is not built to meet these requirements is config file validation (see schema dir). However I believe that this is important to make it easier to detect problems with changes to our CI/CD pipelines. I also believe that this is less complex and easier to read than re-implementing full config validation and defaulting outside of the jsonschema library.

Remaining items

This is missing the following pieces that I will add in follow up PRs after this gets an initial review:

Tests
A README.md (I will try to add answers to functional questions asked by reviewers to this)
A GitHub Action and/or reusable workflow to run this in our current pipelines

Usage

The tool is intended to be used as follows (GHA wrapper to come):

Config file:

---
# yaml-language-server: $schema=./config.json
logger:
  level: debug
attune:
  authentication:
    mTLS:
      endpoint: https://attune.ci-cd-cluster.tld
      certificateSource:
        teleport:
          workloadIdentity:
            name: workload-id
packageManagers:
  - apt:
      fileSource:
        s3:
          bucket: some-bucket-name
          path: teleport/tags/
      components:
        # NOTE: this will require updating whenever a new major branch is cut.
        # I'm open to suggestions on making this easier... maybe templating of some kind?
        stable/v18: &major_version_files
          - teleport-(amd|arm)64\.deb
          - teleport-updater\.deb
          - other-packages\.deb
        stable/cloud:
          - teleport-amd64\.deb
        stable/rolling: *major_version_files
      # When adding a new supported OS, update the github.com/gravitational/teleport/blob/master/lib/web/scripts/node-join/install.sh script
      # Otherwise, it will keep using the binary installation instead of the deb repo.
      distros:
        # https://wiki.ubuntu.com/Releases for details
        ubuntu:
          - xenial # 16.04 LTS
          - bionic # 18.04 LTS
          - focal # 20.04 LTS
          - jammy # 22.04 LTS
          - noble # 24.04 LTS
          - plucky # 25.04
        # See https://wiki.debian.org/DebianReleases#Production_Releases for details
        debian:
          - bullseye # 11
          - bookworm # 12
          - trixie # 13
          - forky # 14

Run it:

# Install tools
apt install aws-cli teleport attune

# Platform-specific auth
aws sso login
tsh login

# Run the tool
oprt2 -c /path/to/config.yaml

socket-security · 2025-09-25T20:21:54Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	github.com/aws/smithy-go@v1.23.0
	github.com/aws/aws-sdk-go-v2/config@v1.31.10
	github.com/aws/aws-sdk-go-v2/service/s3@v1.88.2
	github.com/aws/aws-sdk-go-v2/feature/s3/manager@v1.19.8
	github.com/kaptinlin/jsonschema@v0.4.14
	golang.org/x/sync@v0.17.0

View full report

zmb3 · 2025-09-25T23:20:37Z

+	} else {
+		// Reset the length to 0, but don't release the allocated space
+		soh.buffer = soh.buffer[:0]
+		cmd.Stdout = bytes.NewBuffer(soh.buffer)


Can we just make soh.buffer a bytes.Buffer itself? This form of NewBuffer is meant for when you need a buffer with some initial contents (which we don't have since we just cleared out any existing content).

zmb3 · 2025-09-25T23:21:08Z

Is this meant to be committed?

zmb3 · 2025-09-25T23:22:05Z

@@ -0,0 +1,13 @@
+FROM mcr.microsoft.com/vscode/devcontainers/go:1.25
+
+RUN go install gotest.tools/gotestsum@latest && \


Do we need to install @latest here? Looks like we pin the version of gotestsum elsewhere in this PR.

zmb3 · 2025-09-25T23:26:53Z

+
+	logger, err := config.GetLogger(c.Logger)
+	if err != nil {
+		return fmt.Errorf("failed to create logger")


Suggested change

return fmt.Errorf("failed to create logger")

return errors.New("failed to create logger")

For a static error message prefer errors.New over Errorf.

zmb3 · 2025-09-25T23:29:49Z

+	// Ensure that the cleanup hook is always run. This is important to avoid leaking Attune credentials.
+	if closer, ok := authenticator.(commandrunner.CleanupHook); ok {
+		defer func() {
+			cleanupCtx, cancel := context.WithTimeout(context.Background(), time.Minute)


Are you intentionally wrapping context.Background() instead of using ctx here?

If so, context.WithoutCancel(ctx) is another option worth considering. It's a bit more explicit.

zmb3 · 2025-09-25T23:53:15Z

+)
+
+// EnvVarHook provides an easy way to set an environment variable on every
+// command invokation.


Suggested change

// command invokation.

// command invocation.

zmb3 · 2025-09-25T23:59:52Z

+// isOPRT2ConfigValid validates that the provided config file contents match the JSON schema
+// for [OPRT2] config. Returns true if the config is valid, false otherwise. Records error
+// information to stderr.
+func isOPRT2ConfigValid(configFileAsJSON []byte) bool {


Generally library code should return some error type (with as much detail as you'd like) and leave it to the caller to decide whether to write the detail to stderr or do something else with it.

zmb3 · 2025-09-26T00:03:24Z

+// read and write functions will error, stopping the copy.
+// This retains all properties of [io.Copy], including support for [io.WriterTo] and
+// [io.ReaderFrom].
+func Copy(ctx context.Context, dst io.Writer, src io.Reader) (int64, error) {


This feels like an antipattern to me.

// If the context is cancelled, calls to
// read and write functions will error, stopping the copy.

How are you satisfying this? It looks like me like the context is only checked before the read or write operation begins, but as soon as it starts we stop respecting context cancelation and enter a blocking call.

zmb3 · 2025-09-26T00:06:02Z

+	return errors.Join(cleanupErrs...)
+}
+
+// ListItemsWithPrefix returns a list of items in the storage backend that match the given prefix.


Suggested change

// ListItemsWithPrefix returns a list of items in the storage backend that match the given prefix.

// ListItems returns a list of items in the storage backend.

zmb3 · 2025-09-26T00:12:00Z

+				keys := slices.Collect(maps.Keys(uniqueMatchers))
+				slices.Sort(keys)


Suggested change

keys := slices.Collect(maps.Keys(uniqueMatchers))

slices.Sort(keys)

keys := slices.Sorted(maps.Keys(uniqueMatchers))

r0mant

@fheinecke Two requests:

+4K LoC is a lot to review. Please split this PR into several logical parts that are more manageable. Workflows, Makefiles, configs, tool itself can likely be split as well into several logical parts.
I only glanced through the changeset so far but it all seems very complex: contextualcopy.io, io.go, contextlock.go, mutexmap.go, providers, some schema generator? Why is all of this needed, can the implementation be simplified before we spend time on the review? I'm pretty sure even Teleport codebase doesn't have this many wrappers over standard library functions and primitives, I'm certain we can do just fine without any of those for the tool that's just supposed to auth with Teleport and call an API to push packages.

I encourage you to start very simple and massively simplify the implementation, ensure the tool has bare minimum and essential stuff (Start simple and iterate). We can always add complexity when needed. Thanks.

fheinecke · 2025-09-26T19:13:50Z

Please split this PR into several logical parts that are more manageable. Workflows, Makefiles, configs, tool itself can likely be split as well into several logical parts.

Sure, I can do this.

Why is all of this needed, can the implementation be simplified before we spend time on the review?
We can always add complexity when needed.

As mentioned in the PR body the complexity of this is stemming from the following business requirements set prior to writing the first line of code for this project:

It must be able to run both locally and in CI/CD pipelines
- This affects where packages will be pulled from as well as how Attune authentication is handled
It must run significantly faster than our current process
- This affects what code needs to run concurrently
It must support our disaster recovery (rebuilding the entire prod repo from scratch)
- This affects everything related to supporting a longer runtime, like credential renewal
It must not require running through a Teleport-backed proxy (e.g. tsh proxy app attune)
- This necessitates basically building the TLS proxy implementation
It must "nicely" support both being ran as a CLI tool with our current CI/CD pipelines, and should support being used as a library for our replacement of GHA (whatever this ends up looking like)
- This necessitates writing modular, reusable code

Yes, all of this could be simplified, but only if we either:

Cut back on the business asks, or
Shifted the complexity elsewhere (like fixing the Teleport product issues with uploading lots of large files quickly)

Is there something specific that you'd like me to change here?

I'm pretty sure even Teleport codebase doesn't have this many wrappers over standard library functions and primitives

Most if not all the contents of these files are here because:

There are either long-standing open requests for these against the Go stdlib
There were previously open requests for these against the Go stdlib and they were closed with the recommendation that they be implemented by users outside of the stdlib

I can link to these if you like. Basically all of them are here because the Go encourages/requires the use of a context.Context to cancel blocking requests, but the Go standard library is missing support for this in several critical places. Without this:

Secrets will be leaked in several cases, such as SIGINT/SIGKILL being sent to the program from the environment (CI or local user).
The program will not exit gracefully, and will hang in most error cases

fheinecke · 2025-09-26T19:15:06Z

@zmb3 I'll copy over and address your review comments on the split/follow up PRs

Add OPRT2

3f30347

fheinecke requested a review from a team as a code owner September 25, 2025 20:20

fheinecke requested review from aadc-dev, doggydogworld and r0mant September 25, 2025 20:20

github-advanced-security AI found potential problems Sep 25, 2025

View reviewed changes

Comment thread tools/oprt2/pkg/attunehooks/gpg/archive/archiveprovider.go Dismissed

Fred Heinecke added 4 commits September 25, 2025 20:33

fix linter issues

d12e447

Run go generate using host GOOS/GOARCH

68c2760

makefile fix

92acd33

Upgrade gotestsum due to 1.25 incompatibility

398dfe6

zmb3 reviewed Sep 26, 2025

View reviewed changes

r0mant reviewed Sep 26, 2025

View reviewed changes

fheinecke closed this Sep 26, 2025

This was referenced Sep 26, 2025

Add OPRT2 dev/release tooling #403

Merged

OPRT2 skeleton #407

Merged

fheinecke mentioned this pull request Oct 21, 2025

Add APT publishing to OPRT2 #409

Merged

This was referenced Nov 6, 2025

Implement OPRT2 file managers (s3, local) #415

Merged

Implement GPG providers (archive, key ID) #420

Merged

Add Attune authenticators (token, mTLS) #421

Open

		@@ -0,0 +1,13 @@
		FROM mcr.microsoft.com/vscode/devcontainers/go:1.25

		RUN go install gotest.tools/gotestsum@latest && \

	return fmt.Errorf("failed to create logger")
	return errors.New("failed to create logger")

	// ListItemsWithPrefix returns a list of items in the storage backend that match the given prefix.
	// ListItems returns a list of items in the storage backend.

		keys := slices.Collect(maps.Keys(uniqueMatchers))
		slices.Sort(keys)

	keys := slices.Collect(maps.Keys(uniqueMatchers))
	slices.Sort(keys)
	keys := slices.Sorted(maps.Keys(uniqueMatchers))

Uh oh!

Conversation

fheinecke commented Sep 25, 2025

Summary

Goals

Remaining items

Usage

Uh oh!

socket-security Bot commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

r0mant left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fheinecke commented Sep 26, 2025

Uh oh!

fheinecke commented Sep 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

socket-security Bot commented Sep 25, 2025 •

edited

Loading

r0mant left a comment •

edited

Loading