Skip to content

Commit

Permalink
fix: properly handle from ghcr.io
Browse files Browse the repository at this point in the history
It returns 404 only when trying to pull a layer, but not when we get the
manifest.

Refactor all cases when we try to handle registry errors to use a common
function.

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
  • Loading branch information
smira committed Nov 30, 2023
1 parent f36ab82 commit f82ff73
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 49 deletions.
12 changes: 3 additions & 9 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2023-11-02T09:56:23Z by kres latest.
# Generated on 2023-11-28T14:31:58Z by kres latest.

name: default
concurrency:
Expand All @@ -22,17 +22,16 @@ jobs:
permissions:
actions: read
contents: write
issues: read
packages: write
pull-requests: read
runs-on:
- self-hosted
- generic
if: (!startsWith(github.head_ref, 'renovate/') && !startsWith(github.head_ref, 'dependabot/'))
outputs:
labels: ${{ steps.workflow-run-info.outputs.pullRequestLabels }}
services:
buildkitd:
image: moby/buildkit:v0.12.2
image: moby/buildkit:v0.12.3
options: --privileged
ports:
- 1234:1234
Expand Down Expand Up @@ -96,11 +95,6 @@ jobs:
TEST_FLAGS: -test.schematic-service-repository=registry.dev.siderolabs.io/image-factory/schematic -test.installer-external-repository=registry.dev.siderolabs.io/siderolabs -test.installer-internal-repository=registry.dev.siderolabs.io/siderolabs -test.cache-repository=registry.dev.siderolabs.io/image-factory/cache
run: |
make integration
- name: Retrieve workflow info
id: workflow-run-info
uses: potiuk/get-workflow-origin@v1_5
with:
token: ${{ secrets.GITHUB_TOKEN }}
- name: Generate Checksums
if: startsWith(github.ref, 'refs/tags/')
run: |
Expand Down
19 changes: 10 additions & 9 deletions .github/workflows/slack-notify.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2023-11-02T09:56:23Z by kres latest.
# Generated on 2023-11-21T16:57:47Z by kres latest.

name: slack-notify
"on":
Expand All @@ -14,14 +14,15 @@ jobs:
runs-on:
- self-hosted
- generic
if: ${{ github.event.workflow_run.conclusion != 'skipped' }}
if: github.event.workflow_run.conclusion != 'skipped'
steps:
- name: Retrieve Workflow Run Info
id: retrieve-workflow-run-info
uses: potiuk/get-workflow-origin@v1_5
with:
sourceRunId: ${{ github.event.workflow_run.id }}
token: ${{ secrets.GITHUB_TOKEN }}
- name: Get PR number
id: get-pr-number
if: github.event.workflow_run.event == 'pull_request'
env:
GH_TOKEN: ${{ github.token }}
run: |
echo pull_request_number=$(gh pr view -R ${{ github.repository }} ${{ github.event.workflow_run.head_repository.owner.login }}:${{ github.event.workflow_run.head_branch }} --json number --jq .number) >> $GITHUB_OUTPUT
- name: Slack Notify
uses: slackapi/slack-github-action@v1
with:
Expand All @@ -38,7 +39,7 @@ jobs:
"fields": [
{
"type": "mrkdwn",
"text": "${{ github.event.workflow_run.event == 'pull_request' && format('*Pull Request:* {0} (`{1}`)\n<{2}/pull/{3}|{4}>', github.repository, github.ref_name, github.event.repository.html_url, steps.retrieve-workflow-run-info.outputs.pullRequestNumber, github.event.workflow_run.display_title) || format('*Build:* {0} (`{1}`)\n<{2}/commit/{3}|{4}>', github.repository, github.ref_name, github.event.repository.html_url, github.sha, github.event.workflow_run.display_title) }}"
"text": "${{ github.event.workflow_run.event == 'pull_request' && format('*Pull Request:* {0} (`{1}`)\n<{2}/pull/{3}|{4}>', github.repository, github.ref_name, github.event.repository.html_url, steps.get-pr-number.outputs.pull_request_number, github.event.workflow_run.display_title) || format('*Build:* {0} (`{1}`)\n<{2}/commit/{3}|{4}>', github.repository, github.ref_name, github.event.repository.html_url, github.sha, github.event.workflow_run.display_title) }}"
},
{
"type": "mrkdwn",
Expand Down
5 changes: 5 additions & 0 deletions .kres.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,8 @@ spec:
kind: service.CodeCov
spec:
targetThreshold: 25 # the actual coverage is much higher and reported from the integration test
---
# see https://github.com/golang/go/issues/64112
kind: golang.GoVulnCheck
spec:
disabled: true
9 changes: 2 additions & 7 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2023-11-02T09:56:23Z by kres latest.
# Generated on 2023-11-30T10:18:19Z by kres 902f3bd-dirty.

ARG TOOLCHAIN

FROM alpine:3.18 AS base-image-image-factory

# runs markdownlint
FROM docker.io/node:20.8.0-alpine3.18 AS lint-markdown
FROM docker.io/node:21.1.0-alpine3.18 AS lint-markdown
WORKDIR /src
RUN npm i -g markdownlint-cli@0.37.0
RUN npm i sentences-per-line@0.2.1
Expand Down Expand Up @@ -87,11 +87,6 @@ COPY .golangci.yml .
ENV GOGC 50
RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/root/.cache/golangci-lint --mount=type=cache,target=/go/pkg golangci-lint run --config .golangci.yml

# runs govulncheck
FROM base AS lint-govulncheck
WORKDIR /src
RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/go/pkg govulncheck ./...

# runs unit-tests with race detector
FROM base AS unit-tests-race
WORKDIR /src
Expand Down
13 changes: 6 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2023-11-02T09:56:23Z by kres latest.
# Generated on 2023-11-30T10:20:41Z by kres 902f3bd-dirty.

# common variables

Expand All @@ -16,13 +16,13 @@ USERNAME ?= siderolabs
REGISTRY_AND_USERNAME ?= $(REGISTRY)/$(USERNAME)
PROTOBUF_GO_VERSION ?= 1.31.0
GRPC_GO_VERSION ?= 1.3.0
GRPC_GATEWAY_VERSION ?= 2.18.0
GRPC_GATEWAY_VERSION ?= 2.18.1
VTPROTOBUF_VERSION ?= 0.5.0
DEEPCOPY_VERSION ?= v0.5.5
GOLANGCILINT_VERSION ?= v1.55.1
GOLANGCILINT_VERSION ?= v1.55.2
GOFUMPT_VERSION ?= v0.5.0
GO_VERSION ?= 1.21.3
GOIMPORTS_VERSION ?= v0.14.0
GO_VERSION ?= 1.21.4
GOIMPORTS_VERSION ?= v0.15.0
GO_BUILDFLAGS ?=
GO_LDFLAGS ?=
CGO_ENABLED ?= 0
Expand Down Expand Up @@ -146,8 +146,7 @@ fmt: ## Formats the source code
go install mvdan.cc/gofumpt@$(GOFUMPT_VERSION) && \
gofumpt -w ."

lint-govulncheck: ## Runs govulncheck linter.
@$(MAKE) target-$@
lint-govulncheck: ## Disabled govulncheck linter.

lint-goimports: ## Runs goimports linter.
@$(MAKE) target-$@
Expand Down
6 changes: 2 additions & 4 deletions internal/asset/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ import (
"github.com/google/go-containerregistry/pkg/v1/mutate"
"github.com/google/go-containerregistry/pkg/v1/partial"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/google/go-containerregistry/pkg/v1/remote/transport"
"github.com/sigstore/cosign/v2/pkg/cosign"
"go.uber.org/zap"

"github.com/siderolabs/image-factory/internal/image/signer"
"github.com/siderolabs/image-factory/internal/regtransport"
)

// registryCache is using OCI registry to cache assets.
Expand All @@ -41,9 +41,7 @@ func (r *registryCache) Get(ctx context.Context, profileID string) (BootAsset, e

desc, err := r.puller.Head(ctx, taggedRef)

var transportError *transport.Error

if errors.As(err, &transportError) && (transportError.StatusCode == http.StatusNotFound || transportError.StatusCode == http.StatusForbidden) {
if regtransport.IsStatusCodeError(err, http.StatusNotFound, http.StatusForbidden) {
// ignore 404/403, it means the image hasn't been pushed yet
return nil, errCacheNotFound
}
Expand Down
7 changes: 2 additions & 5 deletions internal/frontend/http/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package http

import (
"context"
"errors"
"fmt"
"net/http"
"net/url"
Expand All @@ -16,7 +15,6 @@ import (
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/empty"
"github.com/google/go-containerregistry/pkg/v1/mutate"
"github.com/google/go-containerregistry/pkg/v1/remote/transport"
"github.com/google/go-containerregistry/pkg/v1/tarball"
"github.com/julienschmidt/httprouter"
"github.com/sigstore/cosign/v2/pkg/cosign"
Expand All @@ -26,6 +24,7 @@ import (
"github.com/siderolabs/image-factory/internal/artifacts"
"github.com/siderolabs/image-factory/internal/asset"
"github.com/siderolabs/image-factory/internal/profile"
"github.com/siderolabs/image-factory/internal/regtransport"
"github.com/siderolabs/image-factory/pkg/schematic"
)

Expand Down Expand Up @@ -178,9 +177,7 @@ func (f *Frontend) handleManifest(ctx context.Context, w http.ResponseWriter, _
f.logger.Error("error verifying cached image signature", zap.String("image", img.Name()), zap.String("schematic", schematicID), zap.String("version", versionTag), zap.Error(signatureErr))
}

var transportError *transport.Error

if errors.As(err, &transportError) && (transportError.StatusCode == http.StatusNotFound || transportError.StatusCode == http.StatusForbidden) {
if regtransport.IsStatusCodeError(err, http.StatusNotFound, http.StatusForbidden) {
// ignore 404/403, it means the image hasn't been built yet
err = nil
}
Expand Down
29 changes: 29 additions & 0 deletions internal/regtransport/regtransport.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

// Package regtransport implements utilities for interacting with registry transport.
package regtransport

import (
"errors"

"github.com/google/go-containerregistry/pkg/v1/remote/transport"
)

// IsStatusCodeError checks is the error is a transport registry error with one of the given status codes.
func IsStatusCodeError(err error, statusCodes ...int) bool {
var transportError *transport.Error

if !errors.As(err, &transportError) {
return false
}

for _, statusCode := range statusCodes {
if transportError.StatusCode == statusCode {
return true
}
}

return false
}
15 changes: 7 additions & 8 deletions internal/schematic/storage/registry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package registry
import (
"bytes"
"context"
"errors"
"io"
"net/http"

Expand All @@ -18,12 +17,12 @@ import (
"github.com/google/go-containerregistry/pkg/v1/mutate"
"github.com/google/go-containerregistry/pkg/v1/partial"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/google/go-containerregistry/pkg/v1/remote/transport"
"github.com/google/go-containerregistry/pkg/v1/types"
"github.com/opencontainers/go-digest"
"github.com/prometheus/client_golang/prometheus"
"github.com/siderolabs/gen/xerrors"

"github.com/siderolabs/image-factory/internal/regtransport"
"github.com/siderolabs/image-factory/internal/schematic/storage"
)

Expand Down Expand Up @@ -86,9 +85,7 @@ func (s *Storage) Head(ctx context.Context, id string) error {
return nil
}

var transportError *transport.Error

if errors.As(err, &transportError) && transportError.StatusCode == http.StatusNotFound {
if regtransport.IsStatusCodeError(err, http.StatusNotFound) {
return xerrors.NewTaggedf[storage.ErrNotFoundTag]("schematic ID %q not found", id)
}

Expand All @@ -105,9 +102,7 @@ func (s *Storage) Get(ctx context.Context, id string) ([]byte, error) {

layer, err := s.puller.Layer(ctx, s.repository.Digest(digestPrefix+id))
if err != nil {
var transportError *transport.Error

if errors.As(err, &transportError) && transportError.StatusCode == http.StatusNotFound {
if regtransport.IsStatusCodeError(err, http.StatusNotFound) {
return nil, xerrors.NewTaggedf[storage.ErrNotFoundTag]("schematic ID %q not found", id)
}

Expand All @@ -117,6 +112,10 @@ func (s *Storage) Get(ctx context.Context, id string) ([]byte, error) {
// pull the layer
r, err := layer.Compressed()
if err != nil {
if regtransport.IsStatusCodeError(err, http.StatusNotFound) {
return nil, xerrors.NewTaggedf[storage.ErrNotFoundTag]("schematic ID %q not found", id)
}

return nil, err
}

Expand Down

0 comments on commit f82ff73

Please sign in to comment.