Handle AWS secret manager empty response

[migmartri]

We've received this exception

{"level":"error", "msg":"runtime error: invalid memory address or nil pointer dereference: 
goroutine 25579 [running]:
github.com/go-kratos/kratos/v2/middleware/recovery.Recovery.func2.1.1()
	/home/runner/go/pkg/mod/github.com/go-kratos/kratos/v2@v2.5.3/middleware/recovery/recovery.go:53 +0xa7
panic({0x2185140, 0x3b16820})
	/opt/hostedtoolcache/go/1.20.2/x64/src/runtime/panic.go:884 +0x213
github.com/chainloop-dev/chainloop/internal/credentials/aws.(*Manager).ReadCredentials(0xc0006747b0, {0x28df088, 0xc000f01740}, {0xc000e8a660, 0x58}, {0x213dea0, 0xc000e0b2c0})
	/home/runner/work/chainloop/chainloop/internal/credentials/aws/secretmanager.go:123 +0x150
github.com/chainloop-dev/chainloop/internal/blobmanager/oci.(*BackendProvider).FromCredentials(0xc000752e20, {0x28df088, 0xc000f01740}, {0xc000e8a660, 0x58})
	/home/runner/work/chainloop/chainloop/internal/blobmanager/oci/provider.go:39 +0x93
github.com/chainloop-dev/chainloop/app/controlplane/internal/biz.(*OCIRepositoryUseCase).PerformValidation(0xc000704040, {0x28df088, 0xc000f01740}, {0xc0010e5260, 0x24})

The nil pointer is raised here

chainloop/internal/credentials/aws/secretmanager.go

Line 123 in 08cd26f

return json.Unmarshal([]byte(*resp.SecretString), creds)

It seems that for some reason both resp and err were nil?

[buccarel]

It seems that for some reason both resp and err were nil?

When the code checks if err != nil it also does if errors.As(err, &apiErr). If err cannot be asserted as of type smithy.APIError, the function does not return when it should, I believe.

[buccarel]

In fact, in the AWS documentation at this page:
https://aws.github.io/aws-sdk-go-v2/docs/migrating/

the following example can be found:

}
	var apiErr smithy.APIError
	if errors.As(err, &apiErr) {
		code := apiErr.ErrorCode()
		message := apiErr.ErrorMessage()
		// handle error code
		return
	}
	// handle error
	return

The code in Chainloop is implementing the block // handle error code but it's missing the block // handle error. At least, I think this is the cause of the problem.

[migmartri]

hi @buccarel, thanks for looking into this; I think you are right! We don't handle an error that's not an APIError 🤦🏼

chainloop/internal/credentials/aws/secretmanager.go

Lines 112 to 122 in 38505dc

	if err != nil {
	var apiErr smithy.APIError
	if errors.As(err, &apiErr) {
	switch apiErr.ErrorCode() {
	case (&types.ResourceNotFoundException{}).ErrorCode():
	return fmt.Errorf("%w: path=%s", credentials.ErrNotFound, secretID)
	default:
	return err
	}
	}
	}

Would you like to add a patch for this? If so, ptal at the expected commit format in our contribution guide

Thank you!