From 258448aefe2441a327f448a595538bb56e8b2f30 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 10 Jul 2023 12:32:01 +0200 Subject: [PATCH 1/3] fix(cas): return 404 error if artifact does not exist Signed-off-by: Miguel Martinez Trivino --- Makefile | 35 +---------- app/artifact-cas/Makefile | 29 ++------- app/artifact-cas/internal/service/resource.go | 9 ++- app/cli/main.go | 2 +- app/controlplane/Makefile | 33 +++------- common.mk | 62 +++++++++++++++++++ internal/blobmanager/errors.go | 37 +++++++++++ internal/blobmanager/oci/backend.go | 9 ++- 8 files changed, 128 insertions(+), 88 deletions(-) create mode 100644 common.mk create mode 100644 internal/blobmanager/errors.go diff --git a/Makefile b/Makefile index 61433d033..e31e0436d 100644 --- a/Makefile +++ b/Makefile @@ -1,20 +1,8 @@ -VERSION=$(shell git describe --tags --always) +include common.mk +VERSION=$(shell git describe --tags --always) API_PROTO_FILES=$(shell find api -name *.proto) -.PHONY: init -# init env -init: - go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.30.0 - go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.3.0 - go install github.com/envoyproxy/protoc-gen-validate@v1.0.1 - go install github.com/go-kratos/kratos/cmd/protoc-gen-go-errors/v2@latest - go install github.com/go-kratos/kratos/cmd/protoc-gen-go-http/v2@latest - go install github.com/google/wire/cmd/wire@latest - go install github.com/vektra/mockery/v2@v2.20.0 - go install ariga.io/atlas/cmd/atlas@v0.12.0 - go install github.com/bufbuild/buf/cmd/buf@v1.10.0 - .PHONY: api # generate api proto api: @@ -54,22 +42,3 @@ lint: # All tests, both unit and integration test: go test ./... - -# show help -help: - @echo '' - @echo 'Usage:' - @echo ' make [target]' - @echo '' - @echo 'Targets:' - @awk '/^[a-zA-Z\-_0-9]+:/ { \ - helpMessage = match(lastLine, /^# (.*)/); \ - if (helpMessage) { \ - helpCommand = substr($$1, 0, index($$1, ":")-1); \ - helpMessage = substr(lastLine, RSTART + 2, RLENGTH); \ - printf "\033[36m%-22s\033[0m %s\n", helpCommand,helpMessage; \ - } \ - } \ - { lastLine = $$0 }' $(MAKEFILE_LIST) - -.DEFAULT_GOAL := help diff --git a/app/artifact-cas/Makefile b/app/artifact-cas/Makefile index f2b61466d..0b873477b 100644 --- a/app/artifact-cas/Makefile +++ b/app/artifact-cas/Makefile @@ -1,13 +1,13 @@ -VERSION=$(shell git describe --tags --always) +include ../../common.mk .PHONY: config # generate config proto -config: +config: check-buf-tool cd ./internal/conf && buf generate .PHONY: api # generate api proto -api: +api: check-buf-tool cd ./api && buf generate .PHONY: build @@ -30,14 +30,14 @@ test: .PHONY: lint # lint -lint: +lint: check-golangci-lint-tool check-buf-tool golangci-lint run buf lint api buf lint internal/conf .PHONY: generate # generate -generate: +generate: check-wire-tool api config go generate ./... .PHONY: all @@ -45,22 +45,3 @@ generate: all: make config; make generate; - -# show help -help: - @echo '' - @echo 'Usage:' - @echo ' make [target]' - @echo '' - @echo 'Targets:' - @awk '/^[a-zA-Z\-_0-9]+:/ { \ - helpMessage = match(lastLine, /^# (.*)/); \ - if (helpMessage) { \ - helpCommand = substr($$1, 0, index($$1, ":")-1); \ - helpMessage = substr(lastLine, RSTART + 2, RLENGTH); \ - printf "\033[36m%-22s\033[0m %s\n", helpCommand,helpMessage; \ - } \ - } \ - { lastLine = $$0 }' $(MAKEFILE_LIST) - -.DEFAULT_GOAL := help diff --git a/app/artifact-cas/internal/service/resource.go b/app/artifact-cas/internal/service/resource.go index f8238306d..d1a1741c7 100644 --- a/app/artifact-cas/internal/service/resource.go +++ b/app/artifact-cas/internal/service/resource.go @@ -21,6 +21,7 @@ import ( v1 "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" backend "github.com/chainloop-dev/chainloop/internal/blobmanager" sl "github.com/chainloop-dev/chainloop/internal/servicelogger" + "github.com/go-openapi/errors" ) type ResourceService struct { @@ -41,13 +42,15 @@ func (s *ResourceService) Describe(ctx context.Context, req *v1.ResourceServiceD return nil, err } - backend, err := s.backendP.FromCredentials(ctx, info.StoredSecretID) + b, err := s.backendP.FromCredentials(ctx, info.StoredSecretID) if err != nil { return nil, sl.LogAndMaskErr(err, s.log) } - res, err := backend.Describe(ctx, req.Digest) - if err != nil { + res, err := b.Describe(ctx, req.Digest) + if err != nil && backend.IsNotFound(err) { + return nil, errors.NotFound("not found", err.Error()) + } else if err != nil { return nil, sl.LogAndMaskErr(err, s.log) } diff --git a/app/cli/main.go b/app/cli/main.go index ce8ba7d39..b8e919982 100644 --- a/app/cli/main.go +++ b/app/cli/main.go @@ -38,7 +38,7 @@ func main() { } } -// handle predefinided errors and handle types so we can tailor the experience +// handle predefined errors and handle types so we can tailor the experience func errorInfo(err error, logger zerolog.Logger) (string, int) { var msg string exitCode := 1 diff --git a/app/controlplane/Makefile b/app/controlplane/Makefile index cdf0838a4..b8ad1b353 100644 --- a/app/controlplane/Makefile +++ b/app/controlplane/Makefile @@ -1,13 +1,13 @@ -VERSION=$(shell git describe --tags --always) +include ../../common.mk .PHONY: config # generate config proto bindings -config: +config: check-buf-tool cd ./internal/conf && buf generate .PHONY: api # generate api proto bindings -api: +api: check-buf-tool cd ./api && buf generate .PHONY: build @@ -28,13 +28,13 @@ local_db = postgres://postgres:@localhost:5432/controlplane?sslmode=disable .PHONY: migration_apply # run migrations against local db -migration_apply: +migration_apply: check-atlas-tool atlas migrate status --dir ${local_migrations_dir} --url ${local_db} atlas migrate apply --dir ${local_migrations_dir} --url ${local_db} .PHONY: migration_new # generate new migration if needed from the current ent schema -migration_new: +migration_new: check-atlas-tool atlas migrate diff --dir ${local_migrations_dir} --to "ent://internal/data/ent/schema" --dev-url "docker://postgres/15/test?search_path=public" .PHONY: test @@ -49,14 +49,14 @@ test-unit: .PHONY: lint # lint -lint: +lint: check-golangci-lint-tool check-buf-tool buf lint api buf lint internal/conf golangci-lint run .PHONY: generate # generate proto bindings, wire injectors, and ent models -generate: api config +generate: check-wire-tool api config go generate ./... .PHONY: all @@ -69,22 +69,3 @@ all: # Visualize data model visualize-data-model: xdg-open internal/data/ent/schema-viz.html - -# show help -help: - @echo '' - @echo 'Usage:' - @echo ' make [target]' - @echo '' - @echo 'Targets:' - @awk '/^[a-zA-Z\-_0-9]+:/ { \ - helpMessage = match(lastLine, /^# (.*)/); \ - if (helpMessage) { \ - helpCommand = substr($$1, 0, index($$1, ":")-1); \ - helpMessage = substr(lastLine, RSTART + 2, RLENGTH); \ - printf "\033[36m%-22s\033[0m %s\n", helpCommand,helpMessage; \ - } \ - } \ - { lastLine = $$0 }' $(MAKEFILE_LIST) - -.DEFAULT_GOAL := help diff --git a/common.mk b/common.mk new file mode 100644 index 000000000..3eba2349c --- /dev/null +++ b/common.mk @@ -0,0 +1,62 @@ +VERSION=$(shell git describe --tags --always) + +.PHONY: init +# init env +init: + go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.30.0 + go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.3.0 + go install github.com/envoyproxy/protoc-gen-validate@v1.0.1 + go install github.com/go-kratos/kratos/cmd/protoc-gen-go-errors/v2@latest + go install github.com/go-kratos/kratos/cmd/protoc-gen-go-http/v2@latest + go install github.com/google/wire/cmd/wire@latest + go install github.com/vektra/mockery/v2@v2.20.0 + go install ariga.io/atlas/cmd/atlas@v0.12.0 + go install github.com/bufbuild/buf/cmd/buf@v1.10.0 + +# show help +help: + @echo '' + @echo 'Usage:' + @echo ' make [target]' + @echo '' + @echo 'Targets:' + @awk '/^[a-zA-Z\-_0-9]+:/ { \ + helpMessage = match(lastLine, /^# (.*)/); \ + if (helpMessage) { \ + helpCommand = substr($$1, 0, index($$1, ":")-1); \ + helpMessage = substr(lastLine, RSTART + 2, RLENGTH); \ + printf "\033[36m%-22s\033[0m %s\n", helpCommand,helpMessage; \ + } \ + } \ + { lastLine = $$0 }' $(MAKEFILE_LIST) + +.DEFAULT_GOAL := help + +.PHONY: check-goreleaser-tool +check-atlas-tool: + @if ! command -v atlas >/dev/null 2>&1; then \ + echo "altas is not installed. Please run \"make init\" or install the tool manually."; \ + exit 1; \ + fi + +.PHONY: check-wire-tool +check-wire-tool: + @if ! command -v wire >/dev/null 2>&1; then \ + echo "wire is not installed. Please run \"make init\" or install the tool manually."; \ + exit 1; \ + fi + +.PHONY: check-golangci-lint-tool +check-golangci-lint-tool: + @if ! command -v golangci-lint >/dev/null 2>&1; then \ + echo "golangci-lint is not installed. Please run \"make init\" or install the tool manually."; \ + exit 1; \ + fi + +.PHONY: check-buf-tool +check-buf-tool: + @if ! command -v buf >/dev/null 2>&1; then \ + echo "buf is not installed. Please run \"make init\" or install the tool manually."; \ + exit 1; \ + fi + diff --git a/internal/blobmanager/errors.go b/internal/blobmanager/errors.go new file mode 100644 index 000000000..540234015 --- /dev/null +++ b/internal/blobmanager/errors.go @@ -0,0 +1,37 @@ +// +// Copyright 2023 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package backend + +import ( + "errors" + "fmt" +) + +type ErrNotFound struct { + entity string +} + +func NewErrNotFound(entity string) ErrNotFound { + return ErrNotFound{entity} +} + +func (e ErrNotFound) Error() string { + return fmt.Sprintf("%s not found", e.entity) +} + +func IsNotFound(err error) bool { + return errors.As(err, &ErrNotFound{}) +} diff --git a/internal/blobmanager/oci/backend.go b/internal/blobmanager/oci/backend.go index 12059258a..0c297b5d3 100644 --- a/internal/blobmanager/oci/backend.go +++ b/internal/blobmanager/oci/backend.go @@ -29,12 +29,14 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/remote/transport" "github.com/google/go-containerregistry/pkg/v1/static" "github.com/google/go-containerregistry/pkg/v1/types" ocispec "github.com/opencontainers/image-spec/specs-go/v1" pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" + backend "github.com/chainloop-dev/chainloop/internal/blobmanager" ) type Backend struct { @@ -185,6 +187,11 @@ func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, e img, err := remote.Image(ref, remote.WithAuthFromKeychain(b.keychain)) if err != nil { + var e *transport.Error + if errors.As(err, &e) && e.StatusCode == http.StatusNotFound { + return nil, backend.NewErrNotFound("image") + } + return nil, fmt.Errorf("getting image: %w", err) } @@ -197,7 +204,7 @@ func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, e return nil, fmt.Errorf("extracting manifest: %w", err) } - // Valirate image already checked that the manifest has exactly one layer + // Validate image already checked that the manifest has exactly one layer size := manifest.Layers[0].Size filename, ok := manifest.Annotations[ocispec.AnnotationTitle] From 0447a5293cc05abab2d77c80c769f22ed68f5622 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 10 Jul 2023 12:36:09 +0200 Subject: [PATCH 2/3] fix(cas): return 404 error if artifact does not exist Signed-off-by: Miguel Martinez Trivino --- app/artifact-cas/internal/service/resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/artifact-cas/internal/service/resource.go b/app/artifact-cas/internal/service/resource.go index d1a1741c7..910514197 100644 --- a/app/artifact-cas/internal/service/resource.go +++ b/app/artifact-cas/internal/service/resource.go @@ -21,7 +21,7 @@ import ( v1 "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" backend "github.com/chainloop-dev/chainloop/internal/blobmanager" sl "github.com/chainloop-dev/chainloop/internal/servicelogger" - "github.com/go-openapi/errors" + "github.com/go-kratos/kratos/v2/errors" ) type ResourceService struct { From bb4c8c84370e993941a5644a2ae14acb6e40c7e7 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 10 Jul 2023 12:41:36 +0200 Subject: [PATCH 3/3] fix(cas): return 404 error if artifact does not exist Signed-off-by: Miguel Martinez Trivino --- internal/blobmanager/oci/backend_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/blobmanager/oci/backend_test.go b/internal/blobmanager/oci/backend_test.go index 5884e1698..079c52c29 100644 --- a/internal/blobmanager/oci/backend_test.go +++ b/internal/blobmanager/oci/backend_test.go @@ -181,7 +181,7 @@ func (s *testSuite) TestDescribe() { name: "not found", digest: "deadbeef", wantErr: true, - errMsg: "Unknown name", + errMsg: "not found", }, { name: "valid image",