### 2023-05-Sep

https://issues.redhat.com/browse/OCPBUGS-7465
- rough sequence of operations
1. [tests authentication](https://github.com/openshift/oc-mirror/blob/00c48bb918216619941134925565af2f0f8bda34/pkg/cli/mirror/mirror.go#L245C14-L245C14) to src/dest mirror participants (here always reg-to-reg, with local dest reg), using `go-containerregistry`
2. pulls the specified catalogs from config, via `opm render`
3. subsects the catalogs to defined bundle ranges / dependencies according to `imageSetConfig`
4. pushes/pulls relevant blobs/layers/bundles from src reg to fulfill dependency chains, via `oc image mirror`
5. rebuilding catalog image with 3+4
6. pushing catalog image to dest reg, using `go-containerregistry`
- original failure was identity/bearer token expiry in #2, solved by [oc-mirror/pull/678](https://github.com/openshift/oc-mirror/pull/678) (containerd dependency bump for o-reg v.1.29.0 release)

- new failure is in #4

- partial call-chain for oc-mirror #4
- (oc-mirror) [pkg/image/credentials.go](https://github.com/openshift/oc-mirror/blob/1247d55d29b6dc002c6380e80860a1a80e9ea4b9/pkg/image/credentials.go#L15)
- (library-go) [pkg/image/registryclient/client.go](https://github.com/openshift/library-go/blob/d7e7beca5baea998564ed16bd3d7e7dffbc7c01f/pkg/image/registryclient/client.go#L59) (note: just establishes a tracker with defaults)
- (oc) [pkg/cli/image/manifest/dockercredentials](https://github.com/openshift/oc/blob/795bf1a6260847ecfc612da2ab11ea2d6e07da16/pkg/cli/image/manifest/dockercredentials/credential_store_factory.go#L13)
- (docker/registry/registry ... now moby) [registry/auth.go](https://github.com/moby/moby/blob/62725ab2778272df9eaf5f33fb862233ea017f56/registry/auth.go#L43) presuming that the only intent is to invoke CredentialStoreFor later (*)

#### initial reproducer (breaks on listing duration in operator-registry)

cpulimit -l 20 ./bin/oc-mirror list operators --catalog=registry.redhat.io/redhat/certified-operator-index:v4.12 -v 9

#### current reproducer (breaks on pulling blobs from source registry in oc)
- with oc-mirror config:

apiVersion: mirror.openshift.io/v1alpha2
kind: ImageSetConfiguration
storageConfig:
registry:
imageURL: localhost:5000/metadata:latest
skipTLS: true
mirror:
operators:
- catalog: registry.redhat.io/redhat/redhat-operator-index:v4.11
packages:
- name: loki-operator
- name: elasticsearch-operator
- name: odf-operator

- with local destination registry via

docker run -d -p 5000:5000 --restart=always --name registry registry

- and then executing

cpulimit -l 2 ./bin/oc-mirror --config ~/devel/tmp/oc-mirror-config.yaml docker://localhost:5000 --dest-skip-tls --dest-use-http --verbose 9 2>&1 | tee /tmp/oc-mirror-attempt1.txt

### 2023-6-Sep
At [oc/pkg/cli/image/manifest/dockercredentials/credential_store_factory.go](https://github.com/openshift/oc/blob/795bf1a6260847ecfc612da2ab11ea2d6e07da16/pkg/cli/image/manifest/dockercredentials/credential_store_factory.go#L41) is

	return dockerregistry.NewStaticCredentialStore(&types.AuthConfig{
		Username:      authCfg.Username,
		Password:      authCfg.Password,
		IdentityToken: authCfg.IdentityToken,
	})

This is an implementation of the auth.CredentialStore interface

// CredentialStore is an interface for getting credentials for
// a given URL
type CredentialStore interface {
	// Basic returns basic auth for the given URL
	Basic(*url.URL) (string, string)

	// RefreshToken returns a refresh token for the
	// given URL and service
	RefreshToken(*url.URL, string) string

	// SetRefreshToken sets the refresh token if none
	// is provided for the given url and service
	SetRefreshToken(realm *url.URL, service, token string)
}

which [always returns the same values](https://github.com/moby/moby/blob/62725ab2778272df9eaf5f33fb862233ea017f56/registry/auth.go#L43). Indeed, the `SetRefreshToken` method is empty.

There is a RefreshTokenStore in library-go which seems related but not an implementation, and it's not ever used so it's hard to know if it is capable.
There is a BasicCredentials in library-go which implements /part/ of the interface, but it is missing the `RefreshRoken` and `SetRefreshToken` methods.
There is a loginCredentialStore in docker/auth.go which implements the whole interface, but isn't exported.

We could re-implement, if we could figure out where to implement it inside the web of repos:

// based on loginCredentialStore from vendor/github.com/docker/docker/registry/auth.go
type DynamicCredentialStore struct {
	authConfig *types.AuthConfig
}

func NewDynamicCredentialStore(auth *types.AuthConfig) auth.CredentialStore {
	return DynamicCredentialStore{authConfig: auth}
}

func (dcs DynamicCredentialStore) Basic(*url.URL) (string, string) {
	if dcs.authConfig == nil {
		return "", ""
	}
	return dcs.authConfig.Username, dcs.authConfig.Password
}

func (dcs DynamicCredentialStore) RefreshToken(*url.URL, string) string {
	if dcs.authConfig == nil {
		return ""
	}

	return dcs.authConfig.IdentityToken
}

func (dcs DynamicCredentialStore) SetRefreshToken(u *url.URL, service, token string) {
	if dcs.authConfig != nil {
		dcs.authConfig.IdentityToken = token
	}
}

I pulled in a couple of folks who mentioned that oc-mirror seems to avoid the docker client interface and uses classes behind that, which may mean that this won't be sufficient and that any update to avoid the issues created by the StaticCredentialStore would be **substantial**.

Options are to implement the above credential store, or to modify the authorizer to [handle retries](https://github.com/openshift/library-go/blob/d7e7beca5baea998564ed16bd3d7e7dffbc7c01f/pkg/image/registryclient/client.go#L402-L413).

### 2023-8-Sep:
Making the attempt to evaluate the new credential store approach, I:
1. forked oc & oc-mirror and cloned to disk as peers
2. introduced the above `DynamicCredentialStore` as a mod to pkg/cli/image/manifest/dockercredentials/credential_store_factory.go (https://github.com/grokspawn/oc/commit/ad3a196bc0229b7030f884571372218b29a7d89f) [Now in a draft PR to help sharing/convo: https://github.com/openshift/oc/pull/1538]
3. modified oc-mirror's go.mod to `replace github.com/openshift/oc => ../oc`

Initial testing on a mac demonstrated token expiry (with associated 401 error) followed by TLS handshake and continued bearer-based authentication (before failing on the inevitable "I can't rebuild this x86_64 catalog on your arm64" problem :wink: )

**Timestamps from my mac run:**

| event | timestamp |
| -------- | --------
| start | 08 Sep 2023 14:07:06 GMT |
| 401 error | 08 Sep 2023 14:14:49 GMT |
| TLS handshake | 08 Sep 2023 14:14:49 GMT |
| 401 error | 08 Sep 2023 14:49:03 GMT |
| TLS handshake | 08 Sep 2023 14:49:04 GMT |

**Timestamps from my linux/x86_64 run:**
| event | timestamp |
| ----- | --------- |
| start | 12 Sep 2023 20:28:01 GMT |
| 401 error | 12 Sep 2023 20:45:22 GMT |
| TLS handshake | 12 Sep 2023 20:45:22 GMT |
| 401 error | 12 Sep 2023 21:08:15 GMT |
| TLS handshake | 12 Sep 2023 2023 21:08:15 GMT |

*note: each thread independently handshakes in the draft PR*

There's need for a bunch more rigor here, vis a vis:
- some oc-mirror failing utests that didn't seem essential for this effort
- all of the oc test process/review (none of which has happened)
- testing of enough cases to feel confident that all the old Static and all the new Dynamic cases are successful w/o regressions to either oc or oc-mirror

I plan to test some more, on linux/x86_64 to see how that goes, but I feel that we've demonstrated that the failures are out of OPRUN/OPECO team domains, and so it's time for mgmt to engage in a higher-level conversation to prioritize this work against all our teams' backlogs.

So that's as far as I plan to go on this issue until/unless something falls out of those talks.