mirror of
https://github.com/replicatedhq/troubleshoot.git
synced 2026-08-27 00:37:20 +00:00
docs: plan to replace go.podman.io/image with go-containerregistry
Drops the github.com/docker/docker +incompatible chain by migrating pkg/collect/registry.go to google/go-containerregistry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
070fd9bab4
commit
c54375b4b1
@@ -0,0 +1,737 @@
|
||||
# Replace go.podman.io/image with google/go-containerregistry Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Migrate `pkg/collect/registry.go` off `go.podman.io/image/v5` (containers/image) onto `github.com/google/go-containerregistry`, eliminating the transitive `github.com/docker/docker +incompatible` chain that blocks Docker v29 SDK adoption.
|
||||
|
||||
**Architecture:** The `RegistryImages` collector probes whether each requested image exists in its registry by issuing a manifest HEAD. Today this goes through `containers/image` (`alltransports.ParseImageName` → `imageRef.NewImage(ctx, sysCtx)`). We replace that with `name.ParseReference` → `remote.Head(ref, opts...)` from `go-containerregistry`, which already exists in our module graph as an indirect dep. Auth comes from a `kubernetes.io/dockerconfigjson` Secret and is converted to an `authn.Authenticator`. Error classification switches from `errcode.Errors` (distribution/v3) and string matching to `transport.Error.StatusCode`.
|
||||
|
||||
**Tech Stack:**
|
||||
- `github.com/google/go-containerregistry/pkg/name` — image reference parsing
|
||||
- `github.com/google/go-containerregistry/pkg/authn` — authenticator
|
||||
- `github.com/google/go-containerregistry/pkg/v1/remote` — registry HEAD/Get
|
||||
- `github.com/google/go-containerregistry/pkg/v1/remote/transport` — typed errors
|
||||
- Go 1.26.x stdlib `net/http`, `crypto/tls`, `errors`
|
||||
- Existing in-tree: `github.com/pkg/errors`, `k8s.io/klog/v2`, `httptest` for unit tests
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
**Files modified:**
|
||||
- `pkg/collect/registry.go` — full rewrite of imports + `imageExists`, `getImageAuthConfig*`, `isNotFound`. Public surface (`CollectRegistry`, `RegistryImage`, `RegistryInfo`) is unchanged.
|
||||
- `pkg/collect/registry_test.go` — replace `alltransports.ParseImageName` with `name.ParseReference`; add httptest-backed integration tests for `imageExists`.
|
||||
- `go.mod` — promote `github.com/google/go-containerregistry` from indirect to direct require; remove `go.podman.io/image/v5` after `go mod tidy`.
|
||||
- `go.sum` — regenerated by `go mod tidy`.
|
||||
|
||||
**Files NOT touched:**
|
||||
- `pkg/collect/images/` — already uses raw `net/http`, unrelated to this migration.
|
||||
- `pkg/apis/troubleshoot/v1beta2/` — `RegistryImages` spec stays unchanged.
|
||||
- `pkg/collect/collector.go` — wiring (`CollectRegistry{...}`) stays unchanged; only `registry.go` internals move.
|
||||
|
||||
---
|
||||
|
||||
## Pre-flight
|
||||
|
||||
- [ ] **Step 0: Confirm baseline tests pass on `main` rebased branch**
|
||||
|
||||
Run: `make test RUN=TestGetImageAuthConfigFromData`
|
||||
Expected: PASS (4 cases).
|
||||
|
||||
Run: `make test RUN=TestImageExists_ContextDeadlineExceeded`
|
||||
Expected: PASS.
|
||||
|
||||
Run: `go build ./...`
|
||||
Expected: no output, exit 0.
|
||||
|
||||
If any of these fail before we touch anything, stop and investigate — the baseline must be green.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Promote `go-containerregistry` to a direct dependency
|
||||
|
||||
**Files:**
|
||||
- Modify: `go.mod`
|
||||
- Modify: `go.sum`
|
||||
|
||||
`go-containerregistry` is currently a transitive dependency at `v0.21.5`. We promote it to a direct require so `go mod tidy` doesn't drop it later, and so future bumps are visible.
|
||||
|
||||
- [ ] **Step 1: Add direct require**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
go get github.com/google/go-containerregistry@v0.21.5
|
||||
```
|
||||
Expected: `go.mod` now lists `github.com/google/go-containerregistry v0.21.5` in the *non-indirect* `require` block (the `// indirect` comment is removed from that line).
|
||||
|
||||
- [ ] **Step 2: Verify build**
|
||||
|
||||
Run: `go build ./...`
|
||||
Expected: no output, exit 0.
|
||||
|
||||
- [ ] **Step 3: Verify the existing tests still pass**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
make test RUN='TestGetImageAuthConfigFromData|TestImageExists_ContextDeadlineExceeded'
|
||||
```
|
||||
Expected: PASS for both tests.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add go.mod go.sum
|
||||
git commit -m "chore(deps): promote go-containerregistry to direct dependency"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Lock in `imageExists` behavior with httptest-backed tests (against the existing implementation)
|
||||
|
||||
**Files:**
|
||||
- Modify: `pkg/collect/registry_test.go`
|
||||
|
||||
We add four new tests that drive `imageExists` against an httptest registry running on `127.0.0.1`. The current `containers/image` implementation must pass them all — that proves we have a faithful behavior contract before refactoring. Note: the current code uses `DockerInsecureSkipTLSVerify: types.OptionalBoolTrue`, so plain-HTTP `httptest.NewServer` works without certs.
|
||||
|
||||
The four cases:
|
||||
1. **Image exists** → `200` on `/v2/` and `/v2/{name}/manifests/{ref}` → `imageExists` returns `(true, nil)`.
|
||||
2. **Image not found** → `200` on `/v2/`, `404` with a `MANIFEST_UNKNOWN` errcode body on the manifest path → `imageExists` returns `(false, nil)`.
|
||||
3. **Unauthorized** → `200` on `/v2/`, `401` on the manifest path → `imageExists` returns `(false, non-nil error)`.
|
||||
4. **EOF retry** → server closes the connection mid-response on the first two attempts and serves a `200` manifest on the third → `imageExists` returns `(true, nil)`.
|
||||
|
||||
- [ ] **Step 1: Add a helper that builds a fake registry**
|
||||
|
||||
Append to `pkg/collect/registry_test.go` (above existing tests, after the `import` block):
|
||||
|
||||
```go
|
||||
// fakeRegistry is a minimal Docker Registry v2 stand-in for unit testing
|
||||
// imageExists. The handler returns whatever the caller puts in `manifest`
|
||||
// for /v2/{name}/manifests/{ref}. /v2/ is always a 200.
|
||||
type fakeRegistry struct {
|
||||
server *httptest.Server
|
||||
manifest http.HandlerFunc
|
||||
}
|
||||
|
||||
func newFakeRegistry(t *testing.T, manifest http.HandlerFunc) *fakeRegistry {
|
||||
t.Helper()
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("/v2/", func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/v2/" || r.URL.Path == "/v2" {
|
||||
w.Header().Set("Docker-Distribution-Api-Version", "registry/2.0")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
return
|
||||
}
|
||||
manifest(w, r)
|
||||
})
|
||||
srv := httptest.NewServer(mux)
|
||||
t.Cleanup(srv.Close)
|
||||
return &fakeRegistry{server: srv, manifest: manifest}
|
||||
}
|
||||
|
||||
// hostPort strips "http://" from the test server URL, leaving "127.0.0.1:NNNN"
|
||||
// suitable for use as the registry portion of an image reference.
|
||||
func (f *fakeRegistry) hostPort() string {
|
||||
return strings.TrimPrefix(f.server.URL, "http://")
|
||||
}
|
||||
```
|
||||
|
||||
Add `"net/http"`, `"net/http/httptest"`, and `"strings"` to the imports of `registry_test.go` if they're not already present (note: `"net"` and `"time"` are already imported by `TestImageExists_ContextDeadlineExceeded`; `"net/http/httptest"` is the new one).
|
||||
|
||||
- [ ] **Step 2: Run the file to confirm it still compiles before adding test cases**
|
||||
|
||||
Run: `go vet ./pkg/collect/...`
|
||||
Expected: no output, exit 0.
|
||||
|
||||
- [ ] **Step 3: Add the "image exists" test**
|
||||
|
||||
Append to `pkg/collect/registry_test.go`:
|
||||
|
||||
```go
|
||||
func TestImageExists_Found(t *testing.T) {
|
||||
fr := newFakeRegistry(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/vnd.docker.distribution.manifest.v2+json")
|
||||
w.Header().Set("Docker-Content-Digest", "sha256:1111111111111111111111111111111111111111111111111111111111111111")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write([]byte(`{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","config":{"mediaType":"application/vnd.docker.container.image.v1+json","size":1,"digest":"sha256:2222222222222222222222222222222222222222222222222222222222222222"},"layers":[]}`))
|
||||
})
|
||||
|
||||
collector := &v1beta2.RegistryImages{
|
||||
Images: []string{fmt.Sprintf("%s/test:latest", fr.hostPort())},
|
||||
}
|
||||
exists, err := imageExists("default", &rest.Config{}, collector, fmt.Sprintf("%s/test:latest", fr.hostPort()), 5*time.Second)
|
||||
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, exists)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run it to confirm the existing implementation passes**
|
||||
|
||||
Run: `go test ./pkg/collect/ -run TestImageExists_Found -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 5: Add the "not found" test**
|
||||
|
||||
Append:
|
||||
|
||||
```go
|
||||
func TestImageExists_NotFound(t *testing.T) {
|
||||
fr := newFakeRegistry(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
_, _ = w.Write([]byte(`{"errors":[{"code":"MANIFEST_UNKNOWN","message":"manifest unknown"}]}`))
|
||||
})
|
||||
|
||||
collector := &v1beta2.RegistryImages{
|
||||
Images: []string{fmt.Sprintf("%s/test:latest", fr.hostPort())},
|
||||
}
|
||||
exists, err := imageExists("default", &rest.Config{}, collector, fmt.Sprintf("%s/test:latest", fr.hostPort()), 5*time.Second)
|
||||
|
||||
assert.NoError(t, err)
|
||||
assert.False(t, exists)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Confirm it passes**
|
||||
|
||||
Run: `go test ./pkg/collect/ -run TestImageExists_NotFound -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 7: Add the "unauthorized" test**
|
||||
|
||||
Append:
|
||||
|
||||
```go
|
||||
func TestImageExists_Unauthorized(t *testing.T) {
|
||||
fr := newFakeRegistry(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Www-Authenticate", `Basic realm="registry"`)
|
||||
w.WriteHeader(http.StatusUnauthorized)
|
||||
_, _ = w.Write([]byte(`{"errors":[{"code":"UNAUTHORIZED","message":"authentication required"}]}`))
|
||||
})
|
||||
|
||||
collector := &v1beta2.RegistryImages{
|
||||
Images: []string{fmt.Sprintf("%s/test:latest", fr.hostPort())},
|
||||
}
|
||||
exists, err := imageExists("default", &rest.Config{}, collector, fmt.Sprintf("%s/test:latest", fr.hostPort()), 5*time.Second)
|
||||
|
||||
assert.Error(t, err)
|
||||
assert.False(t, exists)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 8: Confirm it passes**
|
||||
|
||||
Run: `go test ./pkg/collect/ -run TestImageExists_Unauthorized -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 9: Add the "EOF retry" test**
|
||||
|
||||
Append:
|
||||
|
||||
```go
|
||||
func TestImageExists_RetriesOnEOF(t *testing.T) {
|
||||
var attempts int32
|
||||
fr := newFakeRegistry(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
n := atomic.AddInt32(&attempts, 1)
|
||||
if n < 3 {
|
||||
// hijack the connection and slam it shut to cause an EOF on the client
|
||||
hj, ok := w.(http.Hijacker)
|
||||
if !ok {
|
||||
t.Fatalf("response writer does not support hijacking")
|
||||
}
|
||||
conn, _, err := hj.Hijack()
|
||||
if err != nil {
|
||||
t.Fatalf("hijack: %v", err)
|
||||
}
|
||||
_ = conn.Close()
|
||||
return
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/vnd.docker.distribution.manifest.v2+json")
|
||||
w.Header().Set("Docker-Content-Digest", "sha256:1111111111111111111111111111111111111111111111111111111111111111")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write([]byte(`{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","config":{"mediaType":"application/vnd.docker.container.image.v1+json","size":1,"digest":"sha256:2222222222222222222222222222222222222222222222222222222222222222"},"layers":[]}`))
|
||||
})
|
||||
|
||||
collector := &v1beta2.RegistryImages{
|
||||
Images: []string{fmt.Sprintf("%s/test:latest", fr.hostPort())},
|
||||
}
|
||||
exists, err := imageExists("default", &rest.Config{}, collector, fmt.Sprintf("%s/test:latest", fr.hostPort()), 5*time.Second)
|
||||
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, exists)
|
||||
assert.GreaterOrEqual(t, atomic.LoadInt32(&attempts), int32(3))
|
||||
}
|
||||
```
|
||||
|
||||
Add `"sync/atomic"` to the imports of `registry_test.go`.
|
||||
|
||||
- [ ] **Step 10: Confirm it passes**
|
||||
|
||||
Run: `go test ./pkg/collect/ -run TestImageExists_RetriesOnEOF -v -timeout 30s`
|
||||
Expected: PASS. (May take ~2s because of the sleep-on-EOF in `imageExists`.)
|
||||
|
||||
- [ ] **Step 11: Run the full registry test set together**
|
||||
|
||||
Run: `go test ./pkg/collect/ -run 'TestImageExists|TestGetImageAuthConfigFromData' -v -timeout 60s`
|
||||
Expected: all six tests PASS.
|
||||
|
||||
- [ ] **Step 12: Commit**
|
||||
|
||||
```bash
|
||||
git add pkg/collect/registry_test.go
|
||||
git commit -m "test(collect): lock in imageExists behavior with httptest registry"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Rewrite `registry.go` and `registry_test.go` to use `go-containerregistry`
|
||||
|
||||
**Files:**
|
||||
- Modify: `pkg/collect/registry.go` (full rewrite of imports + bodies; types/signatures of internal helpers change `types.ImageReference` → `name.Reference`).
|
||||
- Modify: `pkg/collect/registry_test.go` (replace `alltransports.ParseImageName` with `name.ParseReference`).
|
||||
|
||||
This is one atomic commit because the function signatures and the test helpers move together.
|
||||
|
||||
- [ ] **Step 1: Replace the imports block in `pkg/collect/registry.go`**
|
||||
|
||||
Replace the file's current imports (lines 3-25) with:
|
||||
|
||||
```go
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/tls"
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
stderrors "errors"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/google/go-containerregistry/pkg/authn"
|
||||
"github.com/google/go-containerregistry/pkg/name"
|
||||
"github.com/google/go-containerregistry/pkg/v1/remote"
|
||||
"github.com/google/go-containerregistry/pkg/v1/remote/transport"
|
||||
"github.com/pkg/errors"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
|
||||
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/client-go/kubernetes"
|
||||
"k8s.io/client-go/rest"
|
||||
"k8s.io/klog/v2"
|
||||
)
|
||||
```
|
||||
|
||||
The aliased `stderrors` is for `stderrors.As` (because `pkg/errors.As` does exist but using stdlib makes the typed-error path explicit). `pkg/errors` keeps providing `Wrap`, `Wrapf`, `New`, `Errorf`, `Cause`.
|
||||
|
||||
The `distribution/distribution/v3` imports (`errcode`, `registryv2`) are gone — we no longer need them.
|
||||
|
||||
- [ ] **Step 2: Replace the `imageExists` function**
|
||||
|
||||
Replace lines 93-165 of `pkg/collect/registry.go` with:
|
||||
|
||||
```go
|
||||
func imageExists(namespace string, clientConfig *rest.Config, registryCollector *troubleshootv1beta2.RegistryImages, image string, deadline time.Duration) (bool, error) {
|
||||
ref, err := name.ParseReference(image)
|
||||
if err != nil {
|
||||
return false, errors.Wrapf(err, "failed to parse image name %s", image)
|
||||
}
|
||||
|
||||
authConfig, err := getImageAuthConfig(namespace, clientConfig, registryCollector, ref)
|
||||
if err != nil {
|
||||
klog.Errorf("failed to get auth config: %v", err)
|
||||
return false, errors.Wrap(err, "failed to get auth config")
|
||||
}
|
||||
|
||||
insecureTransport := &http.Transport{
|
||||
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
|
||||
}
|
||||
|
||||
if deadline == 0 {
|
||||
deadline = 10 * time.Second
|
||||
}
|
||||
|
||||
var lastErr error
|
||||
for i := 0; i < 3; i++ {
|
||||
err := func() error {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), deadline)
|
||||
defer cancel()
|
||||
|
||||
opts := []remote.Option{
|
||||
remote.WithContext(ctx),
|
||||
remote.WithTransport(insecureTransport),
|
||||
}
|
||||
if authConfig != nil {
|
||||
opts = append(opts, remote.WithAuth(&authn.Basic{
|
||||
Username: authConfig.username,
|
||||
Password: authConfig.password,
|
||||
}))
|
||||
}
|
||||
|
||||
_, err := remote.Head(ref, opts...)
|
||||
return err
|
||||
}()
|
||||
if err == nil {
|
||||
klog.V(2).Infof("image %s exists", image)
|
||||
return true, nil
|
||||
}
|
||||
|
||||
klog.Errorf("failed to get image %s: %v", image, err)
|
||||
|
||||
if stderrors.Is(err, context.DeadlineExceeded) {
|
||||
return false, errors.Wrap(err, "failed to get image manifest")
|
||||
}
|
||||
|
||||
if isNotFound(err) {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
if strings.Contains(err.Error(), "EOF") {
|
||||
lastErr = err
|
||||
time.Sleep(1 * time.Second)
|
||||
continue
|
||||
}
|
||||
|
||||
return false, errors.Wrap(err, "failed to get image manifest")
|
||||
}
|
||||
|
||||
return false, errors.Wrap(lastErr, "failed to retry")
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Update auth helper signatures and the docker.io alias normalization**
|
||||
|
||||
Replace lines 167-196 (`getImageAuthConfig`) of `pkg/collect/registry.go` with:
|
||||
|
||||
```go
|
||||
func getImageAuthConfig(namespace string, clientConfig *rest.Config, registryCollector *troubleshootv1beta2.RegistryImages, imageRef name.Reference) (*registryAuthConfig, error) {
|
||||
if registryCollector.ImagePullSecrets == nil {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
if registryCollector.ImagePullSecrets.Data != nil {
|
||||
config, err := getImageAuthConfigFromData(imageRef, registryCollector.ImagePullSecrets)
|
||||
if err != nil {
|
||||
return nil, errors.Wrap(err, "failed to get auth from data")
|
||||
}
|
||||
return config, nil
|
||||
}
|
||||
|
||||
if registryCollector.ImagePullSecrets.Name != "" {
|
||||
collectorNamespace := registryCollector.Namespace
|
||||
if collectorNamespace == "" {
|
||||
collectorNamespace = namespace
|
||||
}
|
||||
if collectorNamespace == "" {
|
||||
collectorNamespace = "default"
|
||||
}
|
||||
config, err := getImageAuthConfigFromSecret(clientConfig, imageRef, registryCollector.ImagePullSecrets, collectorNamespace)
|
||||
if err != nil {
|
||||
return nil, errors.Wrap(err, "failed to get auth from secret")
|
||||
}
|
||||
return config, nil
|
||||
}
|
||||
|
||||
return nil, errors.New("image pull secret spec is not valid")
|
||||
}
|
||||
```
|
||||
|
||||
Replace lines 198-259 (`getImageAuthConfigFromData`) with:
|
||||
|
||||
```go
|
||||
func getImageAuthConfigFromData(imageRef name.Reference, pullSecrets *v1beta2.ImagePullSecrets) (*registryAuthConfig, error) {
|
||||
if pullSecrets.SecretType != "kubernetes.io/dockerconfigjson" {
|
||||
return nil, errors.Errorf("secret type is not supported: %s", pullSecrets.SecretType)
|
||||
}
|
||||
|
||||
configJsonBase64 := pullSecrets.Data[".dockerconfigjson"]
|
||||
registry := imageRef.Context().RegistryStr()
|
||||
|
||||
configJson, err := base64.StdEncoding.DecodeString(configJsonBase64)
|
||||
if err != nil {
|
||||
return nil, errors.Wrap(err, "failed to decode docker config string")
|
||||
}
|
||||
|
||||
dockerCfgJSON := struct {
|
||||
Auths map[string]struct {
|
||||
Auth string `json:"auth"`
|
||||
Username string `json:"username"`
|
||||
Password string `json:"password"`
|
||||
} `json:"auths"`
|
||||
}{}
|
||||
|
||||
err = json.Unmarshal([]byte(configJson), &dockerCfgJSON)
|
||||
if err != nil {
|
||||
return nil, errors.Wrap(err, "failed to unmarshal config json")
|
||||
}
|
||||
|
||||
auth, ok := dockerCfgJSON.Auths[registry]
|
||||
// go-containerregistry normalizes "docker.io" to "index.docker.io"
|
||||
// (name.DefaultRegistry); many dockerconfigjson files key on "docker.io"
|
||||
// instead. Fall back to the alias so existing user secrets keep working.
|
||||
if !ok && registry == name.DefaultRegistry {
|
||||
auth, ok = dockerCfgJSON.Auths["docker.io"]
|
||||
}
|
||||
if !ok {
|
||||
// Support a mix of public and private images
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// gcr.io auth uses username and password, e.g. username: _json_key, password: <sa_key>
|
||||
if auth.Username != "" && auth.Password != "" {
|
||||
return ®istryAuthConfig{
|
||||
username: auth.Username,
|
||||
password: auth.Password,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// docker.io auth uses auth, e.g. auth: <base64_encoded_username_password>
|
||||
// username and password can't contain colon
|
||||
// at least according to https://github.com/docker/cli/blob/v27.0.3/cli/config/configfile/file.go#L247
|
||||
// fallback to not decode for compatibility
|
||||
authStr := auth.Auth
|
||||
decodedAuth, err := base64.StdEncoding.DecodeString(authStr)
|
||||
if err == nil {
|
||||
authStr = string(decodedAuth)
|
||||
}
|
||||
|
||||
parts := strings.Split(authStr, ":")
|
||||
if len(parts) != 2 {
|
||||
return nil, errors.Errorf("expected 2 parts in the auth string, but found %d", len(parts))
|
||||
}
|
||||
|
||||
authConfig := registryAuthConfig{
|
||||
username: parts[0],
|
||||
password: strings.Trim(parts[1], "\x00"),
|
||||
}
|
||||
|
||||
return &authConfig, nil
|
||||
}
|
||||
```
|
||||
|
||||
Replace lines 261-288 (`getImageAuthConfigFromSecret`) with:
|
||||
|
||||
```go
|
||||
func getImageAuthConfigFromSecret(clientConfig *rest.Config, imageRef name.Reference, pullSecrets *v1beta2.ImagePullSecrets, namespace string) (*registryAuthConfig, error) {
|
||||
ctx := context.Background()
|
||||
|
||||
client, err := kubernetes.NewForConfig(clientConfig)
|
||||
if err != nil {
|
||||
return nil, errors.Wrap(err, "failed to create client from config")
|
||||
}
|
||||
|
||||
secret, err := client.CoreV1().Secrets(namespace).Get(ctx, pullSecrets.Name, metav1.GetOptions{})
|
||||
if err != nil {
|
||||
return nil, errors.Wrap(err, "failed to get secret")
|
||||
}
|
||||
|
||||
foundSecrets := &v1beta2.ImagePullSecrets{
|
||||
Name: secret.Name,
|
||||
SecretType: string(secret.Type),
|
||||
Data: map[string]string{
|
||||
".dockerconfigjson": base64.StdEncoding.EncodeToString(secret.Data[".dockerconfigjson"]),
|
||||
},
|
||||
}
|
||||
|
||||
config, err := getImageAuthConfigFromData(imageRef, foundSecrets)
|
||||
if err != nil {
|
||||
return nil, errors.Wrap(err, "failed to get auth from secret data")
|
||||
}
|
||||
|
||||
return config, nil
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Replace `isNotFound` with the typed-error version**
|
||||
|
||||
Replace lines 290-316 of `pkg/collect/registry.go` with:
|
||||
|
||||
```go
|
||||
// isNotFound returns true if err represents a registry response that says the
|
||||
// requested manifest does not exist. go-containerregistry surfaces these as
|
||||
// *transport.Error with HTTP 404.
|
||||
func isNotFound(err error) bool {
|
||||
var terr *transport.Error
|
||||
if stderrors.As(err, &terr) {
|
||||
return terr.StatusCode == http.StatusNotFound
|
||||
}
|
||||
return false
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Remove the unused `bytes` import if necessary, and verify the file compiles**
|
||||
|
||||
The `bytes` import is still used by `Collect()` (`bytes.NewBuffer(b)` on line 88 of the original file). Leave it.
|
||||
|
||||
Run: `go vet ./pkg/collect/...`
|
||||
Expected: no output, exit 0. If vet complains about unused imports, remove them now.
|
||||
|
||||
- [ ] **Step 6: Update `registry_test.go` imports and helper calls**
|
||||
|
||||
In `pkg/collect/registry_test.go`:
|
||||
|
||||
Replace the import line `"go.podman.io/image/v5/transports/alltransports"` with `"github.com/google/go-containerregistry/pkg/name"`.
|
||||
|
||||
In `TestGetImageAuthConfigFromData`, replace:
|
||||
```go
|
||||
imageRef, err := alltransports.ParseImageName(fmt.Sprintf("docker://%s", test.imageName))
|
||||
```
|
||||
with:
|
||||
```go
|
||||
imageRef, err := name.ParseReference(test.imageName)
|
||||
```
|
||||
|
||||
The `docker://` prefix is dropped — `name.ParseReference` doesn't use scheme prefixes.
|
||||
|
||||
- [ ] **Step 7: Build everything**
|
||||
|
||||
Run: `go build ./...`
|
||||
Expected: no output, exit 0.
|
||||
|
||||
- [ ] **Step 8: Run the full registry test suite**
|
||||
|
||||
Run: `go test ./pkg/collect/ -run 'TestImageExists|TestGetImageAuthConfigFromData' -v -timeout 60s`
|
||||
Expected: all six tests PASS.
|
||||
|
||||
- [ ] **Step 9: Run the package tests to catch any unintended regressions**
|
||||
|
||||
Run: `go test ./pkg/collect/... -timeout 5m`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 10: Commit**
|
||||
|
||||
```bash
|
||||
git add pkg/collect/registry.go pkg/collect/registry_test.go
|
||||
git commit -m "refactor(collect): replace go.podman.io/image with go-containerregistry"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Drop `go.podman.io/image/v5` and tidy
|
||||
|
||||
**Files:**
|
||||
- Modify: `go.mod`
|
||||
- Modify: `go.sum`
|
||||
|
||||
After Task 3 there are no remaining references to `go.podman.io/image`, so it can fall out of the module graph along with the heavy `containers/image` chain (`docker/docker`, `docker/cli`, `docker/distribution`, `containers/libtrust`, `containers/ocicrypt`, `morikuni/aec`, etc.).
|
||||
|
||||
- [ ] **Step 1: Confirm zero source references**
|
||||
|
||||
Run: `grep -rn 'go.podman.io/image\|containers/image' --include='*.go' .`
|
||||
Expected: no output.
|
||||
|
||||
- [ ] **Step 2: Tidy the module graph**
|
||||
|
||||
Run: `go mod tidy`
|
||||
Expected: `go.mod` no longer contains `go.podman.io/image/v5`. The `// indirect` block shrinks (specifically: `github.com/docker/docker`, `github.com/docker/cli`, `github.com/docker/distribution` if no other consumer remained, `github.com/containers/libtrust`, `github.com/containers/ocicrypt`, `github.com/morikuni/aec`, `github.com/moby/sys/atomicwriter`, and others should disappear).
|
||||
|
||||
- [ ] **Step 3: Verify `go.podman.io/image` is gone**
|
||||
|
||||
Run: `grep 'go.podman.io/image' go.mod go.sum`
|
||||
Expected: no output.
|
||||
|
||||
- [ ] **Step 4: Verify `docker/docker` is gone**
|
||||
|
||||
Run: `grep 'docker/docker' go.mod`
|
||||
Expected: only `docker/docker-credential-helpers` references remain (used by `helm`, `distribution/v3`, `go-containerregistry`'s authn keychain). The bare `docker/docker` line should be absent. If it isn't absent, run `go mod why -m github.com/docker/docker` to find the remaining consumer and address it before continuing.
|
||||
|
||||
- [ ] **Step 5: Build**
|
||||
|
||||
Run: `go build ./...`
|
||||
Expected: no output, exit 0.
|
||||
|
||||
- [ ] **Step 6: Full test suite**
|
||||
|
||||
Run: `make test`
|
||||
Expected: PASS. (`make test` includes `generate`, `fmt`, `vet`, plus unit tests.)
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add go.mod go.sum
|
||||
git commit -m "chore(deps): drop go.podman.io/image and the docker/docker chain"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Smoke test against a real registry
|
||||
|
||||
**Files:** none modified — this is exercising the binary.
|
||||
|
||||
The unit tests cover the protocol-level behavior. This step verifies the migrated collector works end-to-end against a real public registry, using a real support-bundle spec.
|
||||
|
||||
- [ ] **Step 1: Build the support-bundle binary**
|
||||
|
||||
Run: `make build`
|
||||
Expected: `bin/support-bundle` and `bin/preflight` produced, exit 0.
|
||||
|
||||
- [ ] **Step 2: Author a minimal spec that exercises the collector**
|
||||
|
||||
Write `/tmp/registry-smoke.yaml`:
|
||||
|
||||
```yaml
|
||||
apiVersion: troubleshoot.sh/v1beta2
|
||||
kind: SupportBundle
|
||||
metadata:
|
||||
name: registry-smoke
|
||||
spec:
|
||||
collectors:
|
||||
- registryImages:
|
||||
collectorName: registry-smoke
|
||||
images:
|
||||
- docker.io/library/alpine:3.20
|
||||
- docker.io/library/this-image-definitely-does-not-exist:latest
|
||||
- quay.io/prometheus/node-exporter:latest
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run the support-bundle**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
./bin/support-bundle --output /tmp/registry-smoke.tar.gz /tmp/registry-smoke.yaml
|
||||
```
|
||||
Expected: exit 0. A bundle is produced at `/tmp/registry-smoke.tar.gz`.
|
||||
|
||||
If your environment requires a kubeconfig, set `KUBECONFIG` to any valid file (a kind cluster suffices); the `RegistryImages` collector does not require cluster access unless the spec references `imagePullSecrets`.
|
||||
|
||||
- [ ] **Step 4: Inspect the registry result inside the bundle**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
tar -xOzf /tmp/registry-smoke.tar.gz '*/registry/registry-smoke.json' | jq .
|
||||
```
|
||||
Expected output shape:
|
||||
```json
|
||||
{
|
||||
"images": {
|
||||
"docker.io/library/alpine:3.20": { "exists": true },
|
||||
"docker.io/library/this-image-definitely-does-not-exist:latest": { "exists": false },
|
||||
"quay.io/prometheus/node-exporter:latest": { "exists": true }
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
If `alpine:3.20` reports `exists: false`, the migration broke happy-path manifest fetching — investigate before proceeding.
|
||||
|
||||
If the non-existent image reports `exists: true` or returns an error in the `error` field, the not-found classification is wrong — re-check `isNotFound`.
|
||||
|
||||
- [ ] **Step 5: No commit needed**
|
||||
|
||||
This step is verification only. If everything passes, you're done.
|
||||
|
||||
---
|
||||
|
||||
## Verification summary
|
||||
|
||||
After all five tasks are committed, the branch should:
|
||||
|
||||
1. Have zero source references to `go.podman.io/image` or `containers/image`.
|
||||
2. Have `go.mod` listing `github.com/google/go-containerregistry` as a direct require, and *no* `github.com/docker/docker` line.
|
||||
3. Pass `make test` (unit tests + vet + fmt).
|
||||
4. Pass the smoke test in Task 5.
|
||||
5. Reduce the indirect dependency count noticeably (eyeball `go.sum` line count before/after — expect a drop of several dozen lines).
|
||||
|
||||
## Rollback
|
||||
|
||||
Each task is one commit. To roll back any task, `git revert <sha>` and `go mod tidy`. Tasks 4 and 5 have no source changes, so a revert of Task 3 is sufficient to restore the original implementation; Task 1 stays in place harmlessly.
|
||||
Reference in New Issue
Block a user