diff --git a/cmds/ocm/commands/ocmcmds/references/get/cmd.go b/cmds/ocm/commands/ocmcmds/references/get/cmd.go index 7e75a1a34a..6a57bb89ef 100644 --- a/cmds/ocm/commands/ocmcmds/references/get/cmd.go +++ b/cmds/ocm/commands/ocmcmds/references/get/cmd.go @@ -78,7 +78,12 @@ func (o *Command) Run() error { if err != nil { return err } - return utils.HandleOutputs(opts, hdlr, utils.ElemSpecs(o.Ids)...) + specs, err := utils.ElemSpecs(o.Ids) + if err != nil { + return err + } + + return utils.HandleOutputs(opts, hdlr, specs...) } //////////////////////////////////////////////////////////////////////////////// diff --git a/cmds/ocm/commands/ocmcmds/resources/download/cmd.go b/cmds/ocm/commands/ocmcmds/resources/download/cmd.go index fe9c1b15d1..4f01326ccf 100644 --- a/cmds/ocm/commands/ocmcmds/resources/download/cmd.go +++ b/cmds/ocm/commands/ocmcmds/resources/download/cmd.go @@ -150,7 +150,12 @@ func (o *Command) Run() error { if err != nil { return err } - return utils.HandleOutputs(opts, hdlr, utils.ElemSpecs(o.Ids)...) + specs, err := utils.ElemSpecs(o.Ids) + if err != nil { + return err + } + + return utils.HandleOutputs(opts, hdlr, specs...) } //////////////////////////////////////////////////////////////////////////////// diff --git a/cmds/ocm/commands/ocmcmds/resources/get/cmd.go b/cmds/ocm/commands/ocmcmds/resources/get/cmd.go index 6ea50d3a1e..56851ad8da 100644 --- a/cmds/ocm/commands/ocmcmds/resources/get/cmd.go +++ b/cmds/ocm/commands/ocmcmds/resources/get/cmd.go @@ -76,7 +76,12 @@ func (o *Command) Run() error { if err != nil { return err } - return utils.HandleOutputs(opts, hdlr, utils.ElemSpecs(o.Ids)...) + specs, err := utils.ElemSpecs(o.Ids) + if err != nil { + return err + } + + return utils.HandleOutputs(opts, hdlr, specs...) } //////////////////////////////////////////////////////////////////////////////// diff --git a/cmds/ocm/commands/ocmcmds/sources/get/cmd.go b/cmds/ocm/commands/ocmcmds/sources/get/cmd.go index feb6579f1d..a9dd57d066 100644 --- a/cmds/ocm/commands/ocmcmds/sources/get/cmd.go +++ b/cmds/ocm/commands/ocmcmds/sources/get/cmd.go @@ -76,7 +76,12 @@ func (o *Command) Run() error { if err != nil { return err } - return utils.HandleOutputs(opts, hdlr, utils.ElemSpecs(o.Ids)...) + specs, err := utils.ElemSpecs(o.Ids) + if err != nil { + return err + } + + return utils.HandleOutputs(opts, hdlr, specs...) } //////////////////////////////////////////////////////////////////////////////// diff --git a/cmds/ocm/pkg/processing/buffer.go b/cmds/ocm/pkg/processing/buffer.go index 7fe2d9e810..3ae7e0fd91 100644 --- a/cmds/ocm/pkg/processing/buffer.go +++ b/cmds/ocm/pkg/processing/buffer.go @@ -12,6 +12,7 @@ import ( "github.com/mandelsoft/logging" "github.com/open-component-model/ocm/cmds/ocm/pkg/data" + "github.com/open-component-model/ocm/pkg/utils/panics" ) type Index = IndexArray @@ -72,6 +73,10 @@ type ProcessingEntry struct { type SubEntries int func NewEntry(i Index, v interface{}, opts ...interface{}) ProcessingEntry { + // If this is caught the only upstream problem would be an empty entry. + // Which is fine if the user understands that it can happen. + defer panics.HandlePanic() + max := -1 sub := 0 valid := true diff --git a/cmds/ocm/pkg/utils/handling.go b/cmds/ocm/pkg/utils/handling.go index 1d77ce311b..44b4a76d8f 100644 --- a/cmds/ocm/pkg/utils/handling.go +++ b/cmds/ocm/pkg/utils/handling.go @@ -44,19 +44,19 @@ func StringElemSpecs(args ...string) []ElemSpec { return r } -func ElemSpecs(list interface{}) []ElemSpec { +func ElemSpecs(list interface{}) ([]ElemSpec, error) { if list == nil { - return nil + return nil, nil } v := reflect.ValueOf(list) if v.Kind() != reflect.Array && v.Kind() != reflect.Slice { - panic("no array") + return nil, fmt.Errorf("kind was not an array but: %s", v.Kind().String()) } r := make([]ElemSpec, v.Len()) for i := 0; i < v.Len(); i++ { r[i] = v.Index(i).Interface().(ElemSpec) } - return r + return r, nil } func HandleArgs(opts *output.Options, handler TypeHandler, args ...string) error { diff --git a/pkg/contexts/datacontext/util.go b/pkg/contexts/datacontext/util.go index 035c720015..749f6f00af 100644 --- a/pkg/contexts/datacontext/util.go +++ b/pkg/contexts/datacontext/util.go @@ -26,7 +26,7 @@ type ElementCopyable[T any] interface { type ElementCreator[T any] func(base ...T) T -func SetupElement[T ElementCopyable[T]](mode BuilderMode, target *T, create ElementCreator[T], def T) { +func SetupElement[T ElementCopyable[T]](mode BuilderMode, target *T, create ElementCreator[T], def T) error { var zero T if *target == zero { switch mode { @@ -41,7 +41,9 @@ func SetupElement[T ElementCopyable[T]](mode BuilderMode, target *T, create Elem case MODE_SHARED: *target = def default: - panic(fmt.Sprintf("invalid context creation mode %s", mode)) + return fmt.Errorf("invalid context creation mode %s", mode) } } + + return nil } diff --git a/pkg/contexts/oci/attrs/cacheattr/attr.go b/pkg/contexts/oci/attrs/cacheattr/attr.go index 6935a110fc..6e4dd296ac 100644 --- a/pkg/contexts/oci/attrs/cacheattr/attr.go +++ b/pkg/contexts/oci/attrs/cacheattr/attr.go @@ -51,7 +51,7 @@ func (a AttributeType) Decode(data []byte, unmarshaller runtime.Unmarshaler) (in if strings.HasPrefix(value, "~"+string(os.PathSeparator)) { home := os.Getenv("HOME") if home == "" { - panic("HOME not set") + return nil, fmt.Errorf("HOME not set") } value = home + value[1:] } diff --git a/pkg/contexts/oci/cpi/flavor_artifact.go b/pkg/contexts/oci/cpi/flavor_artifact.go index 4bce489cee..7d9f669489 100644 --- a/pkg/contexts/oci/cpi/flavor_artifact.go +++ b/pkg/contexts/oci/cpi/flavor_artifact.go @@ -77,7 +77,7 @@ func NewArtifact(access ArtifactSetContainer, defs ...*artdesc.Artifact) (Artifa } state, err := accessobj.NewBlobStateForObject(mode, def, NewArtifactStateHandler()) if err != nil { - panic("oops: " + err.Error()) + return nil, err } p, err := access.NewArtifactProvider(state) diff --git a/pkg/contexts/oci/cpi/flavor_index.go b/pkg/contexts/oci/cpi/flavor_index.go index ba24b82213..9d4f2e56de 100644 --- a/pkg/contexts/oci/cpi/flavor_index.go +++ b/pkg/contexts/oci/cpi/flavor_index.go @@ -5,6 +5,8 @@ package cpi import ( + "fmt" + "github.com/opencontainers/go-digest" "github.com/open-component-model/ocm/pkg/common/accessio" @@ -31,7 +33,7 @@ func NewIndex(access ArtifactSetContainer, defs ...*artdesc.Index) (internal.Ind } state, err := accessobj.NewBlobStateForObject(mode, def, NewIndexStateHandler()) if err != nil { - panic("oops") + return nil, fmt.Errorf("failed to get blob state for object: %w", err) } p, err := access.NewArtifactProvider(state) diff --git a/pkg/contexts/oci/cpi/flavor_manifest.go b/pkg/contexts/oci/cpi/flavor_manifest.go index 37791e80d6..59e39998f0 100644 --- a/pkg/contexts/oci/cpi/flavor_manifest.go +++ b/pkg/contexts/oci/cpi/flavor_manifest.go @@ -6,6 +6,7 @@ package cpi import ( "compress/gzip" + "fmt" "github.com/opencontainers/go-digest" @@ -32,7 +33,7 @@ func NewManifest(access ArtifactSetContainer, defs ...*artdesc.Manifest) (*Manif } state, err := accessobj.NewBlobStateForObject(mode, def, NewManifestStateHandler()) if err != nil { - panic("oops") + return nil, fmt.Errorf("failed to get blob state for object: %w", err) } p, err := access.NewArtifactProvider(state) diff --git a/pkg/contexts/oci/cpi/support/flavor_artifact.go b/pkg/contexts/oci/cpi/support/flavor_artifact.go index 918796b6f7..2e235678fc 100644 --- a/pkg/contexts/oci/cpi/support/flavor_artifact.go +++ b/pkg/contexts/oci/cpi/support/flavor_artifact.go @@ -49,7 +49,7 @@ func NewArtifact(container ArtifactSetContainerImpl, defs ...*artdesc.Artifact) } state, err := accessobj.NewBlobStateForObject(mode, def, cpi.NewArtifactStateHandler()) if err != nil { - panic("oops: " + err.Error()) + return nil, fmt.Errorf("failed to fetch new blob state: %w", err) } return newArtifactImpl(container, state) } diff --git a/pkg/contexts/oci/repositories/ocireg/blobs.go b/pkg/contexts/oci/repositories/ocireg/blobs.go index 6d9ae53df1..185532d110 100644 --- a/pkg/contexts/oci/repositories/ocireg/blobs.go +++ b/pkg/contexts/oci/repositories/ocireg/blobs.go @@ -48,16 +48,22 @@ func NewBlobContainers(ctx cpi.Context, fetcher remotes.Fetcher, pusher resolve. } } -func (c *BlobContainers) Get(mime string) BlobContainer { +func (c *BlobContainers) Get(mime string) (BlobContainer, error) { c.lock.Lock() defer c.lock.Unlock() found := c.mimes[mime] if found == nil { - found = NewBlobContainer(c.cache, mime, c.fetcher, c.pusher) - c.mimes[mime] = found + container, err := NewBlobContainer(c.cache, mime, c.fetcher, c.pusher) + if err != nil { + return nil, err + } + c.mimes[mime] = container + + return container, nil } - return found + + return found, nil } func (c *BlobContainers) Release() error { @@ -78,17 +84,17 @@ func newBlobContainer(mime string, fetcher resolve.Fetcher, pusher resolve.Pushe } } -func NewBlobContainer(cache accessio.BlobCache, mime string, fetcher resolve.Fetcher, pusher resolve.Pusher) BlobContainer { +func NewBlobContainer(cache accessio.BlobCache, mime string, fetcher resolve.Fetcher, pusher resolve.Pusher) (BlobContainer, error) { c := newBlobContainer(mime, fetcher, pusher) if cache == nil { - return c + return c, nil } r, err := accessio.CachedAccess(c, c, cache) if err != nil { - panic(err) + return nil, err } - return r + return r, nil } func (n *blobContainer) GetBlobData(digest digest.Digest) (int64, cpi.DataAccess, error) { diff --git a/pkg/contexts/oci/repositories/ocireg/namespace.go b/pkg/contexts/oci/repositories/ocireg/namespace.go index a9877d3b9e..b136e9fffe 100644 --- a/pkg/contexts/oci/repositories/ocireg/namespace.go +++ b/pkg/contexts/oci/repositories/ocireg/namespace.go @@ -120,7 +120,11 @@ func (n *NamespaceContainer) GetBlobDescriptor(digest digest.Digest) *cpi.Descri func (n *NamespaceContainer) GetBlobData(digest digest.Digest) (int64, cpi.DataAccess, error) { n.repo.ctx.Logger().Debug("getting blob", "digest", digest) - size, acc, err := n.blobs.Get("").GetBlobData(digest) + blob, err := n.blobs.Get("") + if err != nil { + return -1, nil, fmt.Errorf("failed to retrieve blob data: %w", err) + } + size, acc, err := blob.GetBlobData(digest) n.repo.ctx.Logger().Debug("getting blob done", "digest", digest, "size", size, "error", logging.ErrorMessage(err)) return size, acc, err } @@ -128,7 +132,11 @@ func (n *NamespaceContainer) GetBlobData(digest digest.Digest) (int64, cpi.DataA func (n *NamespaceContainer) AddBlob(blob cpi.BlobAccess) error { log := n.repo.ctx.Logger() log.Debug("adding blob", "digest", blob.Digest()) - if _, _, err := n.blobs.Get("").AddBlob(blob); err != nil { + blobData, err := n.blobs.Get("") + if err != nil { + return fmt.Errorf("failed to retrieve blob data: %w", err) + } + if _, _, err := blobData.AddBlob(blob); err != nil { log.Debug("adding blob failed", "digest", blob.Digest(), "error", err.Error()) return fmt.Errorf("unable to add blob: %w", err) } @@ -151,7 +159,11 @@ func (n *NamespaceContainer) GetArtifact(vers string) (cpi.ArtifactAccess, error } return nil, err } - _, acc, err := n.blobs.Get(desc.MediaType).GetBlobData(desc.Digest) + blobData, err := n.blobs.Get(desc.MediaType) + if err != nil { + return nil, fmt.Errorf("failed to retrieve blob data, blob data was empty: %w", err) + } + _, acc, err := blobData.GetBlobData(desc.Digest) if err != nil { return nil, err } @@ -163,18 +175,25 @@ func (n *NamespaceContainer) AddArtifact(artifact cpi.Artifact, tags ...string) if err != nil { return nil, err } + if n.repo.info.Legacy { blob = artdesc.MapArtifactBlobMimeType(blob, true) } + n.repo.ctx.Logger().Debug("adding artifact", "digest", blob.Digest(), "mimetype", blob.MimeType()) - _, _, err = n.blobs.Get(blob.MimeType()).AddBlob(blob) + blobData, err := n.blobs.Get(blob.MimeType()) + if err != nil { + return nil, fmt.Errorf("failed to retrieve blob data: %w", err) + } + + _, _, err = blobData.AddBlob(blob) if err != nil { return nil, err } + if len(tags) > 0 { for _, tag := range tags { - err := n.push(tag, blob) - if err != nil { + if err := n.push(tag, blob); err != nil { return nil, err } } diff --git a/pkg/contexts/ocm/internal/builder.go b/pkg/contexts/ocm/internal/builder.go index 0b7489aec0..162c4fe4c3 100644 --- a/pkg/contexts/ocm/internal/builder.go +++ b/pkg/contexts/ocm/internal/builder.go @@ -189,7 +189,7 @@ func (d *delegatingDecoder) Decode(data []byte, unmarshaler runtime.Unmarshaler) if d.delegate == nil && ociimpl != nil { def, err := ociimpl(d.oci) if err != nil { - panic(fmt.Sprintf("cannot create oci default decoder: %s", err)) + return nil, fmt.Errorf("cannot create oci default decoder: %w", err) } d.delegate = def } diff --git a/pkg/contexts/ocm/utils.go b/pkg/contexts/ocm/utils.go index c9e319bf53..2a2e7d3123 100644 --- a/pkg/contexts/ocm/utils.go +++ b/pkg/contexts/ocm/utils.go @@ -35,7 +35,7 @@ func AssureTargetRepository(session Session, ctx Context, targetref string, opts case string: archive = v default: - panic(fmt.Sprintf("invalid option type %T", o)) + return nil, fmt.Errorf("invalid option type %T", o) } } diff --git a/pkg/signing/handlers/rsa/format.go b/pkg/signing/handlers/rsa/format.go index dfcc4d9725..0219c71919 100644 --- a/pkg/signing/handlers/rsa/format.go +++ b/pkg/signing/handlers/rsa/format.go @@ -12,6 +12,7 @@ import ( "fmt" "io" + "github.com/open-component-model/ocm/pkg/errors" "github.com/open-component-model/ocm/pkg/utils" ) @@ -55,18 +56,24 @@ func GetPrivateKey(key interface{}) (*rsa.PrivateKey, error) { } func WriteKeyData(key interface{}, w io.Writer) error { - block := PemBlockForKey(key) + block, err := PemBlockForKey(key) + if err != nil { + return err + } return pem.Encode(w, block) } func KeyData(key interface{}) ([]byte, error) { buf := bytes.NewBuffer(nil) - block := PemBlockForKey(key) - err := pem.Encode(buf, block) + block, err := PemBlockForKey(key) + if err != nil { + return nil, err + } + err = pem.Encode(buf, block) return buf.Bytes(), err } -func PemBlockForKey(priv interface{}, gen ...bool) *pem.Block { +func PemBlockForKey(priv interface{}, gen ...bool) (*pem.Block, error) { switch k := priv.(type) { case *rsa.PublicKey: if utils.Optional(gen...) { @@ -74,13 +81,13 @@ func PemBlockForKey(priv interface{}, gen ...bool) *pem.Block { if err != nil { panic(err) } - return &pem.Block{Type: "PUBLIC KEY", Bytes: bytes} + return &pem.Block{Type: "PUBLIC KEY", Bytes: bytes}, nil } - return &pem.Block{Type: "RSA PUBLIC KEY", Bytes: x509.MarshalPKCS1PublicKey(k)} + return &pem.Block{Type: "RSA PUBLIC KEY", Bytes: x509.MarshalPKCS1PublicKey(k)}, nil case *rsa.PrivateKey: - return &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(k)} + return &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(k)}, nil default: - panic("invalid key") + return nil, errors.ErrInvalid("key") } } diff --git a/pkg/utils/panics/panics.go b/pkg/utils/panics/panics.go new file mode 100644 index 0000000000..da386cd081 --- /dev/null +++ b/pkg/utils/panics/panics.go @@ -0,0 +1,98 @@ +// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. +// +// SPDX-License-Identifier: Apache-2.0 + +package panics + +import ( + "fmt" + "runtime" + "strings" + + "github.com/open-component-model/ocm/pkg/errors" + "github.com/open-component-model/ocm/pkg/logging" +) + +var ReallyCrash = true + +type PanicHandler func(interface{}) error + +var PanicHandlers = []PanicHandler{logHandler} + +// PropagatePanicAsError is intended to be called via defer to +// map a recovered panic to an error and propagate it to the +// error return value of the calling function. +// Use: defer panics.PropagatePanicAsError(&err, false) +// It incorporates the originally returned error by the surrounding function +// and all errors provided by the panic handlers plus the mapped recovered panic. +func PropagatePanicAsError(errp *error, callStandardHandlers bool, additionalHandlers ...PanicHandler) { + if r := recover(); r != nil { + list := errors.ErrList().Add(mapRecovered(r)) + + if callStandardHandlers { + // gather errors from standard handler + for _, fn := range PanicHandlers { + list.Add(fn(r)) + } + } + + // add errors from explicit handlers + for _, fn := range additionalHandlers { + list.Add(fn(r)) + } + + *errp = list.Result() + } +} + +func HandlePanic(additionalHandlers ...PanicHandler) { + if r := recover(); r != nil { + for _, fn := range PanicHandlers { + _ = fn(r) + } + + for _, fn := range additionalHandlers { + _ = fn(r) + } + + if ReallyCrash { + panic(r) + } + } +} + +// RegisterPanicHandler adds handlers to the panic handler. +func RegisterPanicHandler(handler PanicHandler) { + PanicHandlers = append(PanicHandlers, handler) +} + +func mapRecovered(r interface{}) error { + if err, ok := r.(error); ok { + return err + } else { + // Same as stdlib http server code. Manually allocate stack trace buffer size + // to prevent excessively large logs + const size = 64 << 10 + stacktrace := make([]byte, size) + stacktrace = stacktrace[:runtime.Stack(stacktrace, false)] + + stack := string(stacktrace) + lines := strings.Split(stack, "\n") + offset := 1 + for offset < len(lines) && !strings.HasPrefix(lines[offset], "panic(") { + offset++ + } + if offset < len(lines) { + stack = strings.Join(append(lines[:1], lines[offset:]...), "\n") + } + if _, ok := r.(string); ok { + return fmt.Errorf("Observed a panic: %#v\n%s", r, stack) + } + return fmt.Errorf("Observed a panic: %#v (%v)\n%s", r, r, stack) + } +} + +func logHandler(r interface{}) error { + logging.Logger().Error(mapRecovered(r).Error()) + return nil +} diff --git a/pkg/utils/panics/panics_test.go b/pkg/utils/panics/panics_test.go new file mode 100644 index 0000000000..6f14e21e96 --- /dev/null +++ b/pkg/utils/panics/panics_test.go @@ -0,0 +1,63 @@ +// SPDX-FileCopyrightText: 2023 SAP SE or an SAP affiliate company and Open Component Model contributors. +// +// SPDX-License-Identifier: Apache-2.0 + +package panics_test + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/open-component-model/ocm/pkg/utils/panics" +) + +func caller(topanic interface{}, outerr error, handlers ...panics.PanicHandler) (err error) { + defer panics.PropagatePanicAsError(&err, false, handlers...) + + err = outerr + callee(topanic) + return +} + +func callee(topanic interface{}) { + if topanic != nil { + panic(topanic) + } +} + +var _ = Describe("catch panics", func() { + It("propagates catched panic", func() { + defer func() { + Expect(recover()).To(BeNil()) + }() + err := caller("exception", nil) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(MatchRegexp(`(?s)Observed a panic: "exception" +goroutine [0-9]* \[running\]: +panic.*$`)) + }) + + It("propagates catched panic with handlers", func() { + defer func() { + Expect(recover()).To(BeNil()) + }() + err := caller("exception", nil, func(any) error { return fmt.Errorf("handler") }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(MatchRegexp(`(?s){Observed a panic: "exception" +goroutine [0-9]* \[running\]: +panic.* +, handler}$`)) + }) + + It("regular error", func() { + defer func() { + Expect(recover()).To(BeNil()) + }() + err := caller(nil, fmt.Errorf("exception")) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("exception")) + }) + +}) diff --git a/pkg/utils/panics/suite_test.go b/pkg/utils/panics/suite_test.go new file mode 100644 index 0000000000..a7eb26cb8b --- /dev/null +++ b/pkg/utils/panics/suite_test.go @@ -0,0 +1,17 @@ +// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. +// +// SPDX-License-Identifier: Apache-2.0 + +package panics_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestConfig(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Panic Handling Test Suite") +} diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 1a6bd8f203..9858a9d3d3 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -294,7 +294,7 @@ func GetOptionFlag(list ...bool) bool { return OptionalDefaultedBool(len(list) == 0, list...) } -// Must expects a result to be provided without error. +// Must expect a result to be provided without error. func Must[T any](o T, err error) T { if err != nil { panic(fmt.Errorf("expected a %T, but got error %w", o, err))