已合并
fix: resolve issues #192, #193, and #194 #189
xianxian-sauce创建于 9月1日
fix: resolve issues #192, #193, and #194 #189
已合并
共 9 个文件变更+76-17
| @@ -149,15 +149,12 @@ func (s *Store) Delete(ctx context.Context, rawName string) error { | |||
| 149 | nsctx := containerdclient.NewNamespaceContext(ctx) | 149 | nsctx := containerdclient.NewNamespaceContext(ctx) |
| 150 | record, err := s.images.Get(nsctx, recordName) | 150 | record, err := s.images.Get(nsctx, recordName) |
| 151 | if err != nil { | 151 | if err != nil { |
| 152 | - if errdefs.IsNotFound(err) { | ||
| 153 | - return nil | ||
| 154 | - } | ||
| 155 | return translateError("get Template image record", err) | 152 | return translateError("get Template image record", err) |
| 156 | } | 153 | } |
| 157 | if record.Labels[schemaLabel] != schemaVersion { | 154 | if record.Labels[schemaLabel] != schemaVersion { |
| 158 | return conchtemplate.ErrNotFound.Wrap(fmt.Errorf("image record %s is not a Template", recordName)) | 155 | return conchtemplate.ErrNotFound.Wrap(fmt.Errorf("image record %s is not a Template", recordName)) |
| 159 | } | 156 | } |
| 160 | - if err := s.images.Delete(nsctx, recordName, images.DeleteTarget(&record.Target)); err != nil && !errdefs.IsNotFound(err) { | 157 | + if err := s.images.Delete(nsctx, recordName, images.DeleteTarget(&record.Target)); err != nil { |
| 161 | return translateError("delete Template image record", err) | 158 | return translateError("delete Template image record", err) |
| 162 | } | 159 | } |
| 163 | return nil | 160 | return nil |
| @@ -105,8 +105,8 @@ func TestStoreCRUDUsesNamedImageRecord(t *testing.T) { | |||
| 105 | if err := store.Delete(ctx, name); err != nil { | 105 | if err := store.Delete(ctx, name); err != nil { |
| 106 | t.Fatalf("Delete() error = %v", err) | 106 | t.Fatalf("Delete() error = %v", err) |
| 107 | } | 107 | } |
| 108 | - if err := store.Delete(ctx, name); err != nil { | 108 | + if err := store.Delete(ctx, name); !errors.Is(err, conchtemplate.ErrNotFound) { |
| 109 | - t.Fatalf("idempotent Delete() error = %v", err) | 109 | + t.Fatalf("second Delete() error = %v, want ErrNotFound", err) |
| 110 | } | 110 | } |
| 111 | if _, err := store.Get(ctx, name); !errors.Is(err, conchtemplate.ErrNotFound) { | 111 | if _, err := store.Get(ctx, name); !errors.Is(err, conchtemplate.ErrNotFound) { |
| 112 | t.Fatalf("Get() after Delete error = %v, want ErrNotFound", err) | 112 | t.Fatalf("Get() after Delete error = %v, want ErrNotFound", err) |
| @@ -177,14 +177,17 @@ func TestNewRejectsInvalidYAML(t *testing.T) { | |||
| 177 | } | 177 | } |
| 178 | } | 178 | } |
| 179 | 179 | ||
| 180 | -func TestNewAllowsMissingConfigFile(t *testing.T) { | 180 | +func TestNewRejectsMissingExplicitConfigFile(t *testing.T) { |
| 181 | configPath := t.TempDir() + "/missing.yaml" | 181 | configPath := t.TempDir() + "/missing.yaml" |
| 182 | c, err := New(Options{ConfigPath: configPath}) | 182 | c, err := New(Options{ConfigPath: configPath}) |
| 183 | - if err != nil { | 183 | + if err == nil { |
| 184 | - t.Fatalf("New() error = %v", err) | 184 | + t.Fatal("New() error = nil") |
| 185 | } | 185 | } |
| 186 | - if c == nil { | 186 | + if c != nil { |
| 187 | - t.Fatal("New() client = nil") | 187 | + t.Fatalf("New() client = %#v, want nil", c) |
| 188 | + } | ||
| 189 | + if !errors.Is(err, os.ErrNotExist) { | ||
| 190 | + t.Fatalf("New() error = %v, want os.ErrNotExist", err) | ||
| 188 | } | 191 | } |
| 189 | } | 192 | } |
| 190 | 193 | ||
| @@ -531,8 +531,8 @@ func TestCheckpointSandboxBuildsConsecutiveTemplateLineage(t *testing.T) { | |||
| 531 | if err := svc.RemoveTemplate(ctx, t1Name); err != nil { | 531 | if err := svc.RemoveTemplate(ctx, t1Name); err != nil { |
| 532 | t.Fatalf("RemoveTemplate(t1) error = %v", err) | 532 | t.Fatalf("RemoveTemplate(t1) error = %v", err) |
| 533 | } | 533 | } |
| 534 | - if err := svc.RemoveTemplate(ctx, t1Name); err != nil { | 534 | + if err := svc.RemoveTemplate(ctx, t1Name); !errors.Is(err, conchtemplate.ErrNotFound) { |
| 535 | - t.Fatalf("RemoveTemplate(t1) second call error = %v", err) | 535 | + t.Fatalf("RemoveTemplate(t1) second call error = %v, want ErrNotFound", err) |
| 536 | } | 536 | } |
| 537 | if _, err := svc.Templates.Get(ctx, t1Name); !errors.Is(err, conchtemplate.ErrNotFound) { | 537 | if _, err := svc.Templates.Get(ctx, t1Name); !errors.Is(err, conchtemplate.ErrNotFound) { |
| 538 | t.Fatalf("GetTemplate(t1) after removal error = %v, want ErrNotFound", err) | 538 | t.Fatalf("GetTemplate(t1) after removal error = %v, want ErrNotFound", err) |
| @@ -166,10 +166,6 @@ func LoadConfig(configPath string) (*Config, error) { | |||
| 166 | // Open the file once so the permission check applies to the same file that is read. | 166 | // Open the file once so the permission check applies to the same file that is read. |
| 167 | file, err := os.Open(configPath) | 167 | file, err := os.Open(configPath) |
| 168 | if err != nil { | 168 | if err != nil { |
| 169 | - if os.IsNotExist(err) { | ||
| 170 | - // Config file doesn't exist, use default | ||
| 171 | - return DefaultConfig(), nil | ||
| 172 | - } | ||
| 173 | return nil, fmt.Errorf("failed to read config file: %w", err) | 169 | return nil, fmt.Errorf("failed to read config file: %w", err) |
| 174 | } | 170 | } |
| 175 | defer file.Close() | 171 | defer file.Close() |
| @@ -99,6 +99,9 @@ func handleSignals(ctx context.Context, cancel context.CancelFunc, s *Daemon) { | |||
| 99 | } | 99 | } |
| 100 | 100 | ||
| 101 | func New(cfg *config.Config) (*Daemon, error) { | 101 | func New(cfg *config.Config) (*Daemon, error) { |
| 102 | + if err := ensureProcessTempDir(); err != nil { | ||
| 103 | + return nil, err | ||
| 104 | + } | ||
| 102 | ctx, cancel := context.WithCancel(context.Background()) | 105 | ctx, cancel := context.WithCancel(context.Background()) |
| 103 | 106 | ||
| 104 | s := &Daemon{ | 107 | s := &Daemon{ |
| @@ -219,6 +222,14 @@ func New(cfg *config.Config) (*Daemon, error) { | |||
| 219 | return s, nil | 222 | return s, nil |
| 220 | } | 223 | } |
| 221 | 224 | ||
| 225 | +func ensureProcessTempDir() error { | ||
| 226 | + tmpDir := os.TempDir() | ||
| 227 | + if err := os.MkdirAll(tmpDir, 0o700); err != nil { | ||
| 228 | + return fmt.Errorf("create process temp directory %s: %w", tmpDir, err) | ||
| 229 | + } | ||
| 230 | + return nil | ||
| 231 | +} | ||
| 232 | + | ||
| 222 | func (s *Daemon) routes() { | 233 | func (s *Daemon) routes() { |
| 223 | s.router.HandleFunc("POST /api/v1/events/webhooks", s.handleCreateWebhook) | 234 | s.router.HandleFunc("POST /api/v1/events/webhooks", s.handleCreateWebhook) |
| 224 | s.router.HandleFunc("GET /api/v1/events/webhooks", s.handleListWebhooks) | 235 | s.router.HandleFunc("GET /api/v1/events/webhooks", s.handleListWebhooks) |
| @@ -0,0 +1,19 @@ | |||
| 1 | +package daemon | ||
| 2 | + | ||
| 3 | +import ( | ||
| 4 | + "os" | ||
| 5 | + "path/filepath" | ||
| 6 | + "testing" | ||
| 7 | +) | ||
| 8 | + | ||
| 9 | +func TestEnsureProcessTempDirCreatesMissingDirectory(t *testing.T) { | ||
| 10 | + tmpDir := filepath.Join(t.TempDir(), "conch", "tmp") | ||
| 11 | + t.Setenv("TMPDIR", tmpDir) | ||
| 12 | + | ||
| 13 | + if err := ensureProcessTempDir(); err != nil { | ||
| 14 | + t.Fatalf("ensureProcessTempDir() error = %v", err) | ||
| 15 | + } | ||
| 16 | + if info, err := os.Stat(tmpDir); err != nil || !info.IsDir() { | ||
| 17 | + t.Fatalf("temp directory stat = %#v, error = %v", info, err) | ||
| 18 | + } | ||
| 19 | +} | ||
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | "net/http" | 7 | "net/http" |
| 8 | 8 | ||
| 9 | remoteerrors "github.com/containerd/containerd/v2/core/remotes/errors" | 9 | remoteerrors "github.com/containerd/containerd/v2/core/remotes/errors" |
| 10 | + containerdreference "github.com/containerd/containerd/v2/pkg/reference" | ||
| 10 | "github.com/containerd/errdefs" | 11 | "github.com/containerd/errdefs" |
| 11 | "github.com/openeuler/Conch/internal/apperror" | 12 | "github.com/openeuler/Conch/internal/apperror" |
| 12 | ) | 13 | ) |
| @@ -38,6 +39,11 @@ func translateRegistryError(err error) error { | |||
| 38 | if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { | 39 | if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { |
| 39 | return err | 40 | return err |
| 40 | } | 41 | } |
| 42 | + if errors.Is(err, containerdreference.ErrInvalid) || | ||
| 43 | + errors.Is(err, containerdreference.ErrObjectRequired) || | ||
| 44 | + errors.Is(err, containerdreference.ErrHostnameRequired) { | ||
| 45 | + return ErrInvalidArgument.Wrap(err) | ||
| 46 | + } | ||
| 41 | if errdefs.IsNotFound(err) { | 47 | if errdefs.IsNotFound(err) { |
| 42 | return ErrRegistryNotFound.Wrap(err) | 48 | return ErrRegistryNotFound.Wrap(err) |
| 43 | } | 49 | } |
| @@ -7,7 +7,9 @@ import ( | |||
| 7 | "net/http" | 7 | "net/http" |
| 8 | "testing" | 8 | "testing" |
| 9 | 9 | ||
| 10 | + "github.com/containerd/containerd/v2/core/remotes/docker" | ||
| 10 | remoteerrors "github.com/containerd/containerd/v2/core/remotes/errors" | 11 | remoteerrors "github.com/containerd/containerd/v2/core/remotes/errors" |
| 12 | + containerdreference "github.com/containerd/containerd/v2/pkg/reference" | ||
| 11 | "github.com/containerd/errdefs" | 13 | "github.com/containerd/errdefs" |
| 12 | "github.com/openeuler/Conch/internal/apperror" | 14 | "github.com/openeuler/Conch/internal/apperror" |
| 13 | ) | 15 | ) |
| @@ -64,6 +66,31 @@ func TestTranslateRegistryErrorClassifiesResolverNotFound(t *testing.T) { | |||
| 64 | } | 66 | } |
| 65 | } | 67 | } |
| 66 | 68 | ||
| 69 | +func TestTranslateRegistryErrorClassifiesInvalidReference(t *testing.T) { | ||
| 70 | + resolver := docker.NewResolver(docker.ResolverOptions{}) | ||
| 71 | + for _, tc := range []struct { | ||
| 72 | + reference string | ||
| 73 | + cause error | ||
| 74 | + }{ | ||
| 75 | + {reference: "12345", cause: containerdreference.ErrObjectRequired}, | ||
| 76 | + {reference: "registry.example/team/image", cause: containerdreference.ErrObjectRequired}, | ||
| 77 | + {reference: "https://registry.example/team/image:latest", cause: containerdreference.ErrInvalid}, | ||
| 78 | + {reference: "/team/image:latest", cause: containerdreference.ErrHostnameRequired}, | ||
| 79 | + } { | ||
| 80 | + _, _, cause := resolver.Resolve(context.Background(), tc.reference) | ||
| 81 | + if !errors.Is(cause, tc.cause) { | ||
| 82 | + t.Fatalf("Resolve(%q) error = %v, want %v", tc.reference, cause, tc.cause) | ||
| 83 | + } | ||
| 84 | + got := translateRegistryError(cause) | ||
| 85 | + if !errors.Is(got, ErrInvalidArgument) { | ||
| 86 | + t.Fatalf("error = %v, want %s", got, ErrInvalidArgument.Code()) | ||
| 87 | + } | ||
| 88 | + if !errors.Is(got, tc.cause) { | ||
| 89 | + t.Fatalf("error = %v, want cause %v", got, tc.cause) | ||
| 90 | + } | ||
| 91 | + } | ||
| 92 | +} | ||
| 93 | + | ||
| 67 | func TestTranslateRegistryErrorPreservesUnknownFailure(t *testing.T) { | 94 | func TestTranslateRegistryErrorPreservesUnknownFailure(t *testing.T) { |
| 68 | cause := errors.New("content store I/O failed") | 95 | cause := errors.New("content store I/O failed") |
| 69 | err := fmt.Errorf("classify fetched image: %w", cause) | 96 | err := fmt.Errorf("classify fetched image: %w", cause) |