From 430991c39bedafb231e0017277e69ad95cb1d56e Mon Sep 17 00:00:00 2001 From: Ophestra Date: Fri, 15 Aug 2025 00:37:07 +0900 Subject: [PATCH] hst/fs: remove type method Having a method that returns the canonical string representation of its type seemed like a much better idea for an implementation that never made it to staging. Remove it here and clean up marshal type assertions. Signed-off-by: Ophestra --- hst/fs.go | 58 ++++++++++++++++------------------------- hst/fs_test.go | 47 ++++++++++++++------------------- hst/fsbind.go | 3 +-- hst/fsbind_test.go | 2 +- hst/fsephemeral.go | 3 +-- hst/fsephemeral_test.go | 2 +- 6 files changed, 46 insertions(+), 69 deletions(-) diff --git a/hst/fs.go b/hst/fs.go index ceb0aa4..a9ced38 100644 --- a/hst/fs.go +++ b/hst/fs.go @@ -11,8 +11,6 @@ import ( // FilesystemConfig is an abstract representation of a mount point. type FilesystemConfig interface { - // Type returns the type of this mount point. - Type() string // Valid returns whether the configuration is valid. Valid() bool // Target returns the pathname of the mount point in the container. @@ -34,12 +32,8 @@ type FSTypeError string func (f FSTypeError) Error() string { return fmt.Sprintf("invalid filesystem type %q", string(f)) } -// FSImplError is returned when the underlying struct of [FilesystemConfig] does not match -// what [FilesystemConfig.Type] claims to be. -type FSImplError struct { - Type string - Value FilesystemConfig -} +// FSImplError is returned for unsupported implementations of [FilesystemConfig]. +type FSImplError struct{ Value FilesystemConfig } func (f FSImplError) Error() string { implType := reflect.TypeOf(f.Value) @@ -53,57 +47,49 @@ func (f FSImplError) Error() string { } else { name += "nil" } - return fmt.Sprintf("implementation %s is not %s", name, f.Type) + return fmt.Sprintf("implementation %s not supported", name) } // FilesystemConfigJSON is the [json] adapter for [FilesystemConfig]. -type FilesystemConfigJSON struct { - FilesystemConfig -} +type FilesystemConfigJSON struct{ FilesystemConfig } // Valid returns whether the [FilesystemConfigJSON] is valid. func (f *FilesystemConfigJSON) Valid() bool { return f != nil && f.FilesystemConfig != nil && f.FilesystemConfig.Valid() } +// fsType holds the string representation of a [FilesystemConfig]'s concrete type. +type fsType struct { + Type string `json:"type"` +} + func (f *FilesystemConfigJSON) MarshalJSON() ([]byte, error) { if f == nil || f.FilesystemConfig == nil { return nil, ErrFSNull } var v any - t := f.Type() - switch t { - case FilesystemBind: - if ct, ok := f.FilesystemConfig.(*FSBind); !ok { - return nil, FSImplError{t, f.FilesystemConfig} - } else { - v = &struct { - Type string `json:"type"` - *FSBind - }{FilesystemBind, ct} - } + switch cv := f.FilesystemConfig.(type) { + case *FSBind: + v = &struct { + fsType + *FSBind + }{fsType{FilesystemBind}, cv} - case FilesystemEphemeral: - if ct, ok := f.FilesystemConfig.(*FSEphemeral); !ok { - return nil, FSImplError{t, f.FilesystemConfig} - } else { - v = &struct { - Type string `json:"type"` - *FSEphemeral - }{FilesystemEphemeral, ct} - } + case *FSEphemeral: + v = &struct { + fsType + *FSEphemeral + }{fsType{FilesystemEphemeral}, cv} default: - return nil, FSTypeError(t) + return nil, FSImplError{f.FilesystemConfig} } return json.Marshal(v) } func (f *FilesystemConfigJSON) UnmarshalJSON(data []byte) error { - t := new(struct { - Type string `json:"type"` - }) + t := new(fsType) if err := json.Unmarshal(data, &t); err != nil { return err } diff --git a/hst/fs_test.go b/hst/fs_test.go index c44cd3c..2cdcb1e 100644 --- a/hst/fs_test.go +++ b/hst/fs_test.go @@ -28,17 +28,11 @@ func TestFilesystemConfigJSON(t *testing.T) { `{"type":"cat","meow":true}`, `{"fs":{"type":"cat","meow":true},"magic":3236757504}`}, {"bad impl bind", hst.FilesystemConfigJSON{FilesystemConfig: stubFS{"bind"}}, - hst.FSImplError{ - Type: "bind", - Value: stubFS{"bind"}, - }, + hst.FSImplError{Value: stubFS{"bind"}}, "\x00", "\x00"}, {"bad impl ephemeral", hst.FilesystemConfigJSON{FilesystemConfig: stubFS{"ephemeral"}}, - hst.FSImplError{ - Type: "ephemeral", - Value: stubFS{"ephemeral"}, - }, + hst.FSImplError{Value: stubFS{"ephemeral"}}, "\x00", "\x00"}, {"bind", hst.FilesystemConfigJSON{ @@ -66,12 +60,18 @@ func TestFilesystemConfigJSON(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { t.Run("marshal", func(t *testing.T) { + wantErr := tc.wantErr + if errors.As(wantErr, new(hst.FSTypeError)) { + // for unsupported implementation tc + wantErr = hst.FSImplError{Value: stubFS{"cat"}} + } + { d, err := json.Marshal(&tc.want) - if !errors.Is(err, tc.wantErr) { - t.Errorf("Marshal: error = %v, want %v", err, tc.wantErr) + if !errors.Is(err, wantErr) { + t.Errorf("Marshal: error = %v, want %v", err, wantErr) } - if tc.wantErr != nil { + if wantErr != nil { goto checkSMarshal } if string(d) != tc.data { @@ -82,10 +82,10 @@ func TestFilesystemConfigJSON(t *testing.T) { checkSMarshal: { d, err := json.Marshal(&sCheck{tc.want, syscall.MS_MGC_VAL}) - if !errors.Is(err, tc.wantErr) { - t.Errorf("Marshal: error = %v, want %v", err, tc.wantErr) + if !errors.Is(err, wantErr) { + t.Errorf("Marshal: error = %v, want %v", err, wantErr) } - if tc.wantErr != nil { + if wantErr != nil { return } if string(d) != tc.sData { @@ -170,15 +170,15 @@ func TestFSErrors(t *testing.T) { val hst.FilesystemConfig want string }{ - {"nil", nil, "implementation nil is not cat"}, - {"stub", stubFS{"cat"}, "implementation stubFS is not cat"}, - {"*stub", &stubFS{"cat"}, "implementation *stubFS is not cat"}, - {"(*stub)(nil)", (*stubFS)(nil), "implementation *stubFS is not cat"}, + {"nil", nil, "implementation nil not supported"}, + {"stub", stubFS{"cat"}, "implementation stubFS not supported"}, + {"*stub", &stubFS{"cat"}, "implementation *stubFS not supported"}, + {"(*stub)(nil)", (*stubFS)(nil), "implementation *stubFS not supported"}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err := hst.FSImplError{Type: "cat", Value: tc.val} + err := hst.FSImplError{Value: tc.val} if got := err.Error(); got != tc.want { t.Errorf("Error: %q, want %q", got, tc.want) } @@ -191,7 +191,6 @@ type stubFS struct { typeName string } -func (s stubFS) Type() string { return s.typeName } func (s stubFS) Valid() bool { return false } func (s stubFS) Target() *container.Absolute { panic("unreachable") } func (s stubFS) Host() []*container.Absolute { panic("unreachable") } @@ -213,15 +212,9 @@ type fsTestCase struct { str string } -func checkFs(t *testing.T, fstype string, testCases []fsTestCase) { +func checkFs(t *testing.T, testCases []fsTestCase) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - t.Run("type", func(t *testing.T) { - if got := tc.fs.Type(); got != fstype { - t.Errorf("Type: %q, want %q", got, fstype) - } - }) - t.Run("valid", func(t *testing.T) { if got := tc.fs.Valid(); got != tc.valid { t.Errorf("Valid: %v, want %v", got, tc.valid) diff --git a/hst/fsbind.go b/hst/fsbind.go index 18d9cf7..beceecb 100644 --- a/hst/fsbind.go +++ b/hst/fsbind.go @@ -26,8 +26,7 @@ type FSBind struct { Optional bool `json:"optional,omitempty"` } -func (b *FSBind) Type() string { return FilesystemBind } -func (b *FSBind) Valid() bool { return b != nil && b.Src != nil } +func (b *FSBind) Valid() bool { return b != nil && b.Src != nil } func (b *FSBind) Target() *container.Absolute { if !b.Valid() { diff --git a/hst/fsbind_test.go b/hst/fsbind_test.go index 48e710c..ce904df 100644 --- a/hst/fsbind_test.go +++ b/hst/fsbind_test.go @@ -8,7 +8,7 @@ import ( ) func TestFSBind(t *testing.T) { - checkFs(t, "bind", []fsTestCase{ + checkFs(t, []fsTestCase{ {"nil", (*hst.FSBind)(nil), false, nil, nil, nil, ""}, {"full", &hst.FSBind{ diff --git a/hst/fsephemeral.go b/hst/fsephemeral.go index 209b615..0ed5087 100644 --- a/hst/fsephemeral.go +++ b/hst/fsephemeral.go @@ -25,8 +25,7 @@ type FSEphemeral struct { Perm os.FileMode `json:"perm,omitempty"` } -func (e *FSEphemeral) Type() string { return FilesystemEphemeral } -func (e *FSEphemeral) Valid() bool { return e != nil && e.Dst != nil } +func (e *FSEphemeral) Valid() bool { return e != nil && e.Dst != nil } func (e *FSEphemeral) Target() *container.Absolute { if !e.Valid() { diff --git a/hst/fsephemeral_test.go b/hst/fsephemeral_test.go index 320c12f..9286ac4 100644 --- a/hst/fsephemeral_test.go +++ b/hst/fsephemeral_test.go @@ -9,7 +9,7 @@ import ( ) func TestFSEphemeral(t *testing.T) { - checkFs(t, "ephemeral", []fsTestCase{ + checkFs(t, []fsTestCase{ {"nil", (*hst.FSEphemeral)(nil), false, nil, nil, nil, ""}, {"full", &hst.FSEphemeral{