... for Podman to immediately benefit from the updated heuristic.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This commit is contained in:
Miloslav Trmač
2023-02-13 19:57:22 +01:00
parent 1e06c1a47a
commit 86a4c6b65e
28 changed files with 105 additions and 161 deletions

View File

@@ -19,7 +19,7 @@ type Writer struct {
archive *tarfile.Writer
writer io.Closer
// The following state can only be acccessed with the mutex held.
// The following state can only be accessed with the mutex held.
mutex sync.Mutex
hadCommit bool // At least one successful commit has happened
}

View File

@@ -17,26 +17,31 @@ import (
"github.com/sirupsen/logrus"
)
// bodyReaderMinimumProgress is the minimum progress we want to see before we retry
const bodyReaderMinimumProgress = 1 * 1024 * 1024
const (
// bodyReaderMinimumProgress is the minimum progress we consider a good reason to retry
bodyReaderMinimumProgress = 1 * 1024 * 1024
// bodyReaderMSSinceLastRetry is the minimum time since a last retry we consider a good reason to retry
bodyReaderMSSinceLastRetry = 60 * 1_000
)
// bodyReader is an io.ReadCloser returned by dockerImageSource.GetBlob,
// which can transparently resume some (very limited) kinds of aborted connections.
type bodyReader struct {
ctx context.Context
c *dockerClient
path string // path to pass to makeRequest to retry
logURL *url.URL // a string to use in error messages
body io.ReadCloser // The currently open connection we use to read data, or nil if there is nothing to read from / close.
lastRetryOffset int64
offset int64 // Current offset within the blob
ctx context.Context
c *dockerClient
path string // path to pass to makeRequest to retry
logURL *url.URL // a string to use in error messages
firstConnectionTime time.Time
lastSuccessTime time.Time // time.Time{} if N/A
body io.ReadCloser // The currently open connection we use to read data, or nil if there is nothing to read from / close.
lastRetryOffset int64 // -1 if N/A
lastRetryTime time.Time // time.Time{} if N/A
offset int64 // Current offset within the blob
lastSuccessTime time.Time // time.Time{} if N/A
}
// newBodyReader creates a bodyReader for request path in c.
// firstBody is an already correctly opened body for the blob, returing the full blob from the start.
// firstBody is an already correctly opened body for the blob, returning the full blob from the start.
// If reading from firstBody fails, bodyReader may heuristically decide to resume.
func newBodyReader(ctx context.Context, c *dockerClient, path string, firstBody io.ReadCloser) (io.ReadCloser, error) {
logURL, err := c.resolveRequestURL(path)
@@ -44,15 +49,17 @@ func newBodyReader(ctx context.Context, c *dockerClient, path string, firstBody
return nil, err
}
res := &bodyReader{
ctx: ctx,
c: c,
ctx: ctx,
c: c,
path: path,
logURL: logURL,
body: firstBody,
lastRetryOffset: 0,
offset: 0,
firstConnectionTime: time.Now(),
body: firstBody,
lastRetryOffset: -1,
lastRetryTime: time.Time{},
offset: 0,
lastSuccessTime: time.Time{},
}
return res, nil
}
@@ -186,10 +193,11 @@ func (br *bodyReader) Read(p []byte) (int, error) {
return n, fmt.Errorf("%w (after reconnecting, fetching blob: %v)", originalErr, err)
}
logrus.Debugf("Succesfully reconnected to %s", redactedURL)
logrus.Debugf("Successfully reconnected to %s", redactedURL)
consumedBody = true
br.body = res.Body
br.lastRetryOffset = br.offset
br.lastRetryTime = time.Time{}
return n, nil
default:
@@ -198,29 +206,40 @@ func (br *bodyReader) Read(p []byte) (int, error) {
}
}
// millisecondsSince is like time.Since(tm).Milliseconds, but it returns a floating-point value
func millisecondsSince(tm time.Time) float64 {
return float64(time.Since(tm).Nanoseconds()) / 1_000_000.0
// millisecondsSinceOptional is like currentTime.Sub(tm).Milliseconds, but it returns a floating-point value.
// If tm is time.Time{}, it returns math.NaN()
func millisecondsSinceOptional(currentTime time.Time, tm time.Time) float64 {
if tm == (time.Time{}) {
return math.NaN()
}
return float64(currentTime.Sub(tm).Nanoseconds()) / 1_000_000.0
}
// errorIfNotReconnecting makes a heuristic decision whether we should reconnect after err at redactedURL; if so, it returns nil,
// otherwise it returns an appropriate error to return to the caller (possibly augmented with data about the heuristic)
func (br *bodyReader) errorIfNotReconnecting(originalErr error, redactedURL string) error {
totalTime := millisecondsSince(br.firstConnectionTime)
failureTime := math.NaN()
if (br.lastSuccessTime != time.Time{}) {
failureTime = millisecondsSince(br.lastSuccessTime)
}
logrus.Debugf("Reading blob body from %s failed (%#v), decision inputs: lastRetryOffset %d, offset %d, %.3f ms since first connection, %.3f ms since last progress",
redactedURL, originalErr, br.lastRetryOffset, br.offset, totalTime, failureTime)
currentTime := time.Now()
msSinceFirstConnection := millisecondsSinceOptional(currentTime, br.firstConnectionTime)
msSinceLastRetry := millisecondsSinceOptional(currentTime, br.lastRetryTime)
msSinceLastSuccess := millisecondsSinceOptional(currentTime, br.lastSuccessTime)
logrus.Debugf("Reading blob body from %s failed (%#v), decision inputs: total %d @%.3f ms, last retry %d @%.3f ms, last progress @%.3f ms",
redactedURL, originalErr, br.offset, msSinceFirstConnection, br.lastRetryOffset, msSinceLastRetry, msSinceLastSuccess)
progress := br.offset - br.lastRetryOffset
if progress < bodyReaderMinimumProgress {
logrus.Debugf("Not reconnecting to %s because only %d bytes progress made", redactedURL, progress)
return fmt.Errorf("(heuristic tuning data: last retry %d, current offset %d; %.3f ms total, %.3f ms since progress): %w",
br.lastRetryOffset, br.offset, totalTime, failureTime, originalErr)
if progress >= bodyReaderMinimumProgress {
logrus.Infof("Reading blob body from %s failed (%v), reconnecting after %d bytes…", redactedURL, originalErr, progress)
return nil
}
logrus.Infof("Reading blob body from %s failed (%v), reconnecting…", redactedURL, originalErr)
return nil
if br.lastRetryTime == (time.Time{}) || msSinceLastRetry >= bodyReaderMSSinceLastRetry {
if br.lastRetryTime == (time.Time{}) {
logrus.Infof("Reading blob body from %s failed (%v), reconnecting (first reconnection)…", redactedURL, originalErr)
} else {
logrus.Infof("Reading blob body from %s failed (%v), reconnecting after %.3f ms…", redactedURL, originalErr, msSinceLastRetry)
}
return nil
}
logrus.Debugf("Not reconnecting to %s: insufficient progress %d / time since last retry %.3f ms", redactedURL, progress, msSinceLastRetry)
return fmt.Errorf("(heuristic tuning data: total %d @%.3f ms, last retry %d @%.3f ms, last progress @ %.3f ms): %w",
br.offset, msSinceFirstConnection, br.lastRetryOffset, msSinceLastRetry, msSinceLastSuccess, originalErr)
}
// Close implements io.ReadCloser

View File

@@ -985,7 +985,7 @@ func (c *dockerClient) getBlob(ctx context.Context, ref dockerReference, info ty
return reconnectingReader, blobSize, nil
}
// getOCIDescriptorContents returns the contents a blob spcified by descriptor in ref, which must fit within limit.
// getOCIDescriptorContents returns the contents a blob specified by descriptor in ref, which must fit within limit.
func (c *dockerClient) getOCIDescriptorContents(ctx context.Context, ref dockerReference, desc imgspecv1.Descriptor, maxSize int, cache types.BlobInfoCache) ([]byte, error) {
// Note that this copies all kinds of attachments: attestations, and whatever else is there,
// not just signatures. We leave the signature consumers to decide based on the MIME type.

View File

@@ -742,7 +742,7 @@ func layerMatchesSigstoreSignature(layer imgspecv1.Descriptor, mimeType string,
}
// putBlobBytesAsOCI uploads a blob with the specified contents, and returns an appropriate
// OCI descriptior.
// OCI descriptor.
func (d *dockerImageDestination) putBlobBytesAsOCI(ctx context.Context, contents []byte, mimeType string, options private.PutBlobOptions) (imgspecv1.Descriptor, error) {
blobDigest := digest.FromBytes(contents)
info, err := d.PutBlobWithOptions(ctx, bytes.NewReader(contents),

View File

@@ -8,8 +8,8 @@
// domain := domain-component ['.' domain-component]* [':' port-number]
// domain-component := /([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])/
// port-number := /[0-9]+/
// path-component := alpha-numeric [separator alpha-numeric]*
// alpha-numeric := /[a-z0-9]+/
// path-component := alphanumeric [separator alphanumeric]*
// alphanumeric := /[a-z0-9]+/
// separator := /[_.]|__|[-]*/
//
// tag := /[\w][\w.-]{0,127}/

View File

@@ -10,7 +10,7 @@ import (
)
// FromReference returns a types.ImageCloser implementation for the default instance reading from reference.
// If reference poitns to a manifest list, .Manifest() still returns the manifest list,
// If reference points to a manifest list, .Manifest() still returns the manifest list,
// but other methods transparently return data from an appropriate image instance.
//
// The caller must call .Close() on the returned ImageCloser.

View File

@@ -21,7 +21,6 @@ import (
"github.com/imdario/mergo"
"github.com/sirupsen/logrus"
"golang.org/x/exp/slices"
"golang.org/x/net/http2"
"gopkg.in/yaml.v3"
)
@@ -931,14 +930,14 @@ func tlsCacheGet(config *restConfig) (http.RoundTripper, error) {
Proxy: newProxierWithNoProxyCIDR(http.ProxyFromEnvironment),
TLSHandshakeTimeout: 10 * time.Second,
TLSClientConfig: tlsConfig,
Dial: (&net.Dialer{
DialContext: (&net.Dialer{
Timeout: 30 * time.Second,
KeepAlive: 30 * time.Second,
}).Dial,
}).DialContext,
}
// Allow clients to disable http2 if needed.
if s := os.Getenv("DISABLE_HTTP2"); len(s) == 0 {
_ = http2.ConfigureTransport(t)
t.ForceAttemptHTTP2 = true
}
return t, nil
}

View File

@@ -200,7 +200,7 @@ func streamChunksFromFile(streams chan io.ReadCloser, errs chan error, file io.R
defer file.Close()
for _, c := range chunks {
// Always seek to the desired offest; that way we dont need to care about the consumer
// Always seek to the desired offset; that way we dont need to care about the consumer
// not reading all of the chunk, or about the position going backwards.
if _, err := file.Seek(int64(c.Offset), io.SeekStart); err != nil {
errs <- err

View File

@@ -30,7 +30,7 @@ var (
// Zstd compression.
Zstd = internal.NewAlgorithm(types.ZstdAlgorithmName, types.ZstdAlgorithmName,
[]byte{0x28, 0xb5, 0x2f, 0xfd}, ZstdDecompressor, zstdCompressor)
// ZstdChunked is a Zstd compresion with chunk metadta which allows random access to individual files.
// ZstdChunked is a Zstd compression with chunk metadta which allows random access to individual files.
ZstdChunked = internal.NewAlgorithm(types.ZstdChunkedAlgorithmName, types.ZstdAlgorithmName, /* Note: InternalUnstableUndocumentedMIMEQuestionMark is not ZstdChunkedAlgorithmName */
nil, ZstdDecompressor, compressor.ZstdCompressor)

View File

@@ -105,7 +105,7 @@ func (f *fulcioTrustRoot) verifyFulcioCertificateAtTime(relevantTime time.Time,
// log of approved Fulcio invocations, and its not clear where that would come from, especially human users manually
// logging in using OpenID are not going to maintain a record of those actions.
//
// Also, the SCT does not help reveal _what_ was maliciously signed, nor does it protect against malicous signatures
// Also, the SCT does not help reveal _what_ was maliciously signed, nor does it protect against malicious signatures
// by correctly-issued certificates.
//
// So, pragmatically, the ideal design seem to be to only do signatures from a trusted build system (which is, by definition,

View File

@@ -100,7 +100,7 @@ func WithFulcioAndPreexistingOIDCIDToken(fulcioURL *url.URL, oidcIDToken string)
// WithFulcioAndDeviceAuthorizationGrantOIDC sets up signing to use a short-lived key and a Fulcio-issued certificate
// based on an OIDC ID token obtained using a device authorization grant (RFC 8628).
//
// interactiveOutput must be directly accesible to a human user in real time (i.e. not be just a log file).
// interactiveOutput must be directly accessible to a human user in real time (i.e. not be just a log file).
func WithFulcioAndDeviceAuthorizationGrantOIDC(fulcioURL *url.URL, oidcIssuerURL *url.URL, oidcClientID, oidcClientSecret string,
interactiveOutput io.Writer) internal.Option {
return func(s *internal.SigstoreSigner) error {