From f1eb8b26ef6425596a0996c583be1c60792bc456 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Fri, 3 Nov 2023 15:18:00 +0000 Subject: [PATCH 01/20] refactor(oci): decouple store from config.OCI --- internal/oci/file.go | 54 +++++++++++++++++++++++--- internal/oci/file_test.go | 36 +++++------------ internal/storage/fs/oci/source_test.go | 6 +-- 3 files changed, 59 insertions(+), 37 deletions(-) diff --git a/internal/oci/file.go b/internal/oci/file.go index 38e50c8b61..5e838feb9f 100644 --- a/internal/oci/file.go +++ b/internal/oci/file.go @@ -8,6 +8,7 @@ import ( "io" "io/fs" "path" + "path/filepath" "strings" "time" @@ -31,10 +32,43 @@ type Store struct { local oras.Target } +type StoreOptions struct { + bunchesDir string + auth *struct { + username string + password string + } +} + +func WithBundleDir(dir string) containers.Option[StoreOptions] { + return func(so *StoreOptions) { + so.bunchesDir = dir + } +} + +func WithCredentials(user, pass string) containers.Option[StoreOptions] { + return func(so *StoreOptions) { + so.auth = &struct { + username string + password string + }{ + username: user, + password: pass, + } + } +} + // NewStore constructs and configures an instance of *Store for the provided config -func NewStore(conf *config.OCI) (*Store, error) { - scheme, repository, match := strings.Cut(conf.Repository, "://") +func NewStore(repository string, opts ...containers.Option[StoreOptions]) (*Store, error) { + dir, err := defaultBundleDirectory() + if err != nil { + return nil, err + } + + options := StoreOptions{bunchesDir: dir} + containers.ApplyAll(&options, opts...) + scheme, repository, match := strings.Cut(repository, "://") // support empty scheme as remote and https if !match { repository = scheme @@ -64,11 +98,12 @@ func NewStore(conf *config.OCI) (*Store, error) { } case "flipt": if ref.Registry != "local" { - return nil, fmt.Errorf("unexpected local reference: %q", conf.Repository) + return nil, fmt.Errorf("unexpected local reference: %q", ref) } // build the store once to ensure it is valid - _, err := oci.New(path.Join(conf.BundleDirectory, ref.Repository)) + bundleDir := path.Join(options.bunchesDir, ref.Repository) + _, err := oci.New(bundleDir) if err != nil { return nil, err } @@ -80,7 +115,7 @@ func NewStore(conf *config.OCI) (*Store, error) { // contents changes underneath it // this allows us to change the state with another process and // have the store pickup the changes - return oci.New(path.Join(conf.BundleDirectory, ref.Repository)) + return oci.New(bundleDir) } default: return nil, fmt.Errorf("unexpected repository scheme: %q should be one of [http|https|flipt]", scheme) @@ -280,3 +315,12 @@ func (f FileInfo) IsDir() bool { func (f FileInfo) Sys() any { return nil } + +func defaultBundleDirectory() (string, error) { + dir, err := config.Dir() + if err != nil { + return "", err + } + + return filepath.Join(dir, "bundle"), nil +} diff --git a/internal/oci/file_test.go b/internal/oci/file_test.go index 2bd6554e05..47447d0069 100644 --- a/internal/oci/file_test.go +++ b/internal/oci/file_test.go @@ -12,31 +12,24 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.flipt.io/flipt/internal/config" "oras.land/oras-go/v2" "oras.land/oras-go/v2/content/oci" ) func TestNewStore(t *testing.T) { t.Run("unexpected scheme", func(t *testing.T) { - _, err := NewStore(&config.OCI{ - Repository: "fake://local/something:latest", - }) + _, err := NewStore("fake://local/something:latest") require.EqualError(t, err, `unexpected repository scheme: "fake" should be one of [http|https|flipt]`) }) t.Run("invalid reference", func(t *testing.T) { - _, err := NewStore(&config.OCI{ - Repository: "something:latest", - }) + _, err := NewStore("something:latest") require.EqualError(t, err, `invalid reference: missing repository`) }) t.Run("invalid local reference", func(t *testing.T) { - _, err := NewStore(&config.OCI{ - Repository: "flipt://invalid/something:latest", - }) - require.EqualError(t, err, `unexpected local reference: "flipt://invalid/something:latest"`) + _, err := NewStore("flipt://invalid/something:latest") + require.EqualError(t, err, `unexpected local reference: "invalid/something:latest"`) }) t.Run("valid", func(t *testing.T) { @@ -47,10 +40,7 @@ func TestNewStore(t *testing.T) { "https://remote/something:latest", } { t.Run(repository, func(t *testing.T) { - _, err := NewStore(&config.OCI{ - BundleDirectory: t.TempDir(), - Repository: repository, - }) + _, err := NewStore(repository, WithBundleDir(t.TempDir())) require.NoError(t, err) }) } @@ -62,10 +52,8 @@ func TestStore_Fetch_InvalidMediaType(t *testing.T) { layer("default", `{"namespace":"default"}`, "unexpected.media.type"), ) - store, err := NewStore(&config.OCI{ - BundleDirectory: dir, - Repository: fmt.Sprintf("flipt://local/%s:latest", repo), - }) + repository := fmt.Sprintf("flipt://local/%s:latest", repo) + store, err := NewStore(repository, WithBundleDir(dir)) require.NoError(t, err) ctx := context.Background() @@ -76,10 +64,7 @@ func TestStore_Fetch_InvalidMediaType(t *testing.T) { layer("default", `{"namespace":"default"}`, MediaTypeFliptNamespace+"+unknown"), ) - store, err = NewStore(&config.OCI{ - BundleDirectory: dir, - Repository: fmt.Sprintf("flipt://local/%s:latest", repo), - }) + store, err = NewStore(repository, WithBundleDir(dir)) require.NoError(t, err) _, err = store.Fetch(ctx) @@ -92,10 +77,7 @@ func TestStore_Fetch(t *testing.T) { layer("other", `namespace: other`, MediaTypeFliptNamespace+"+yaml"), ) - store, err := NewStore(&config.OCI{ - BundleDirectory: dir, - Repository: fmt.Sprintf("flipt://local/%s:latest", repo), - }) + store, err := NewStore(fmt.Sprintf("flipt://local/%s:latest", repo), WithBundleDir(dir)) require.NoError(t, err) ctx := context.Background() diff --git a/internal/storage/fs/oci/source_test.go b/internal/storage/fs/oci/source_test.go index 46d0567cbf..c68bb2aee0 100644 --- a/internal/storage/fs/oci/source_test.go +++ b/internal/storage/fs/oci/source_test.go @@ -12,7 +12,6 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.flipt.io/flipt/internal/config" fliptoci "go.flipt.io/flipt/internal/oci" storagefs "go.flipt.io/flipt/internal/storage/fs" "go.uber.org/zap/zaptest" @@ -92,10 +91,7 @@ func testSource(t *testing.T) (*Source, oras.Target) { layer("production", `{"namespace":"production"}`, fliptoci.MediaTypeFliptNamespace), ) - store, err := fliptoci.NewStore(&config.OCI{ - BundleDirectory: dir, - Repository: fmt.Sprintf("flipt://local/%s:latest", repo), - }) + store, err := fliptoci.NewStore(fmt.Sprintf("flipt://local/%s:latest", repo), fliptoci.WithBundleDir(dir)) require.NoError(t, err) source, err := NewSource(zaptest.NewLogger(t), From 5519fd9c012bb2770eec3214999619842623d618 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Fri, 3 Nov 2023 15:20:05 +0000 Subject: [PATCH 02/20] refactor(oci): move directory ensuring into store --- internal/config/storage.go | 14 -------------- internal/oci/file.go | 8 +++++++- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/internal/config/storage.go b/internal/config/storage.go index e5c7a7ac19..bb55aa0682 100644 --- a/internal/config/storage.go +++ b/internal/config/storage.go @@ -3,8 +3,6 @@ package config import ( "errors" "fmt" - "os" - "path/filepath" "time" "github.com/spf13/viper" @@ -63,18 +61,6 @@ func (c *StorageConfig) setDefaults(v *viper.Viper) error { } case string(OCIStorageType): v.SetDefault("store.oci.insecure", false) - - configDir, err := Dir() - if err != nil { - return fmt.Errorf("setting oci default: %w", err) - } - - bundlesDir := filepath.Join(configDir, "bundles") - if err := os.MkdirAll(bundlesDir, 0755); err != nil { - return fmt.Errorf("creating image directory: %w", err) - } - - v.SetDefault("store.oci.bundles_directory", bundlesDir) default: v.SetDefault("storage.type", "database") } diff --git a/internal/oci/file.go b/internal/oci/file.go index 5e838feb9f..619222f7d9 100644 --- a/internal/oci/file.go +++ b/internal/oci/file.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "io/fs" + "os" "path" "path/filepath" "strings" @@ -322,5 +323,10 @@ func defaultBundleDirectory() (string, error) { return "", err } - return filepath.Join(dir, "bundle"), nil + bundlesDir := filepath.Join(dir, "bundles") + if err := os.MkdirAll(bundlesDir, 0755); err != nil { + return "", fmt.Errorf("creating image directory: %w", err) + } + + return bundlesDir, nil } From df16b85db2b4ce918bf67710c44db1609a15369a Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Fri, 3 Nov 2023 17:31:15 +0000 Subject: [PATCH 03/20] feat(cmd/bundle): add new bundle build sub-commands --- cmd/flipt/bundle.go | 67 +++++++++++++ cmd/flipt/main.go | 1 + cmd/flipt/validate.go | 11 ++- internal/oci/file.go | 125 +++++++++++++++++++++-- internal/oci/file_test.go | 49 +++++++-- internal/oci/testdata/.flipt.yml | 3 + internal/oci/testdata/production.yml | 1 + internal/storage/fs/oci/source.go | 2 +- internal/storage/fs/oci/source_test.go | 2 +- internal/storage/fs/snapshot.go | 131 +++++++++++++++++-------- 10 files changed, 330 insertions(+), 62 deletions(-) create mode 100644 cmd/flipt/bundle.go create mode 100644 internal/oci/testdata/.flipt.yml create mode 100644 internal/oci/testdata/production.yml diff --git a/cmd/flipt/bundle.go b/cmd/flipt/bundle.go new file mode 100644 index 0000000000..326a2f931d --- /dev/null +++ b/cmd/flipt/bundle.go @@ -0,0 +1,67 @@ +package main + +import ( + "fmt" + "os" + + "github.com/spf13/cobra" + "go.flipt.io/flipt/internal/containers" + "go.flipt.io/flipt/internal/oci" +) + +type bundleCommand struct { + rootDir string +} + +func newBundleCommand() *cobra.Command { + bundle := &bundleCommand{} + + cmd := &cobra.Command{ + Use: "bundle", + Short: "Manage flipt bundlees", + } + + cmd.AddCommand(&cobra.Command{ + Use: "build", + Short: "build a bundle", + RunE: bundle.build, + Args: cobra.ExactArgs(1), + }) + + return cmd +} + +func (c *bundleCommand) build(cmd *cobra.Command, args []string) error { + logger, cfg, err := buildConfig() + if err != nil { + return err + } + + var opts []containers.Option[oci.StoreOptions] + if cfg := cfg.Storage.OCI; cfg != nil { + if cfg.BundleDirectory != "" { + opts = append(opts, oci.WithBundleDir(cfg.BundleDirectory)) + } + + if cfg.Authentication != nil { + opts = append(opts, oci.WithCredentials( + cfg.Authentication.Username, + cfg.Authentication.Password, + )) + } + } + + store, err := oci.NewStore(logger, fmt.Sprintf("flipt://local/%s", args[0]), opts...) + if err != nil { + return err + } + + bundle, err := store.Build(cmd.Context(), os.DirFS(".")) + if err != nil { + return err + } + + fmt.Println(bundle.Digest) + + return nil +} diff --git a/cmd/flipt/main.go b/cmd/flipt/main.go index 674a83d0f1..9cb39fe6a8 100644 --- a/cmd/flipt/main.go +++ b/cmd/flipt/main.go @@ -140,6 +140,7 @@ func exec() error { rootCmd.AddCommand(newConfigCommand()) rootCmd.AddCommand(newCompletionCommand()) rootCmd.AddCommand(newDocCommand()) + rootCmd.AddCommand(newBundleCommand()) ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/cmd/flipt/validate.go b/cmd/flipt/validate.go index 93b4cfb8f1..ceabf84ce0 100644 --- a/cmd/flipt/validate.go +++ b/cmd/flipt/validate.go @@ -8,7 +8,6 @@ import ( "github.com/spf13/cobra" "go.flipt.io/flipt/internal/cue" "go.flipt.io/flipt/internal/storage/fs" - "go.uber.org/zap" ) type validateCommand struct { @@ -43,11 +42,15 @@ func newValidateCommand() *cobra.Command { } func (v *validateCommand) run(cmd *cobra.Command, args []string) error { - var err error + logger, _, err := buildConfig() + if err != nil { + return err + } + if len(args) == 0 { - _, err = fs.SnapshotFromFS(zap.NewNop(), os.DirFS(".")) + _, err = fs.SnapshotFromFS(logger, os.DirFS(".")) } else { - _, err = fs.SnapshotFromPaths(os.DirFS("."), args...) + _, err = fs.SnapshotFromPaths(logger, os.DirFS("."), args...) } errs, ok := cue.Unwrap(err) diff --git a/internal/oci/file.go b/internal/oci/file.go index 619222f7d9..f74b5a763c 100644 --- a/internal/oci/file.go +++ b/internal/oci/file.go @@ -1,6 +1,7 @@ package oci import ( + "bytes" "context" "encoding/json" "errors" @@ -17,10 +18,14 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" "go.flipt.io/flipt/internal/config" "go.flipt.io/flipt/internal/containers" + "go.flipt.io/flipt/internal/ext" + storagefs "go.flipt.io/flipt/internal/storage/fs" + "go.uber.org/zap" "oras.land/oras-go/v2" "oras.land/oras-go/v2/content" "oras.land/oras-go/v2/content/memory" "oras.land/oras-go/v2/content/oci" + "oras.land/oras-go/v2/errdef" "oras.land/oras-go/v2/registry" "oras.land/oras-go/v2/registry/remote" ) @@ -28,25 +33,33 @@ import ( // Store is a type which can retrieve Flipt feature files from a target repository and reference // Repositories can be local (OCI layout directories on the filesystem) or a remote registry type Store struct { + logger *zap.Logger reference registry.Reference - store func() (oras.ReadOnlyTarget, error) + store func() (oras.Target, error) local oras.Target } +// StoreOptions are used to configure call to NewStore +// This shouldn't be handled directory, instead use one of the function options +// e.g. WithBundleDir or WithCredentials type StoreOptions struct { - bunchesDir string - auth *struct { + bundleDir string + auth *struct { username string password string } } +// WithBundleDir overrides the default bundles directory on the host for storing +// local builds of Flipt bundles func WithBundleDir(dir string) containers.Option[StoreOptions] { return func(so *StoreOptions) { - so.bunchesDir = dir + so.bundleDir = dir } } +// WithCredentials configures username and password credentials used for authenticating +// with remote registries func WithCredentials(user, pass string) containers.Option[StoreOptions] { return func(so *StoreOptions) { so.auth = &struct { @@ -60,13 +73,13 @@ func WithCredentials(user, pass string) containers.Option[StoreOptions] { } // NewStore constructs and configures an instance of *Store for the provided config -func NewStore(repository string, opts ...containers.Option[StoreOptions]) (*Store, error) { +func NewStore(logger *zap.Logger, repository string, opts ...containers.Option[StoreOptions]) (*Store, error) { dir, err := defaultBundleDirectory() if err != nil { return nil, err } - options := StoreOptions{bunchesDir: dir} + options := StoreOptions{bundleDir: dir} containers.ApplyAll(&options, opts...) scheme, repository, match := strings.Cut(repository, "://") @@ -82,6 +95,7 @@ func NewStore(repository string, opts ...containers.Option[StoreOptions]) (*Stor } store := &Store{ + logger: logger, reference: ref, local: memory.New(), } @@ -94,7 +108,7 @@ func NewStore(repository string, opts ...containers.Option[StoreOptions]) (*Stor remote.PlainHTTP = scheme == "http" - store.store = func() (oras.ReadOnlyTarget, error) { + store.store = func() (oras.Target, error) { return remote, nil } case "flipt": @@ -103,20 +117,27 @@ func NewStore(repository string, opts ...containers.Option[StoreOptions]) (*Stor } // build the store once to ensure it is valid - bundleDir := path.Join(options.bunchesDir, ref.Repository) + bundleDir := path.Join(options.bundleDir, ref.Repository) _, err := oci.New(bundleDir) if err != nil { return nil, err } - store.store = func() (oras.ReadOnlyTarget, error) { + store.store = func() (oras.Target, error) { // we recreate the OCI store on every operation for local // because the oci library we use maintains a reference cache // in memory that doesn't get purged when the target directory // contents changes underneath it // this allows us to change the state with another process and // have the store pickup the changes - return oci.New(bundleDir) + store, err := oci.New(bundleDir) + if err != nil { + return nil, err + } + + store.AutoSaveIndex = true + + return store, nil } default: return nil, fmt.Errorf("unexpected repository scheme: %q should be one of [http|https|flipt]", scheme) @@ -251,6 +272,86 @@ func (s *Store) fetchFiles(ctx context.Context, store oras.ReadOnlyTarget, manif return files, nil } +// Bundle is a record of an existing Flipt feature bundle +type Bundle struct { + Digest digest.Digest + Repository string + Tag string + CreatedAt time.Time +} + +// Build bundles the target directory Flipt feature state into the target configured on the Store +// It returns a Bundle which contains metadata regarding the resulting bundle details +func (s *Store) Build(ctx context.Context, src fs.FS) (Bundle, error) { + store, err := s.store() + if err != nil { + return Bundle{}, nil + } + + layers, err := s.buildLayers(ctx, store, src) + if err != nil { + return Bundle{}, nil + } + + desc, err := oras.PackManifest(ctx, store, oras.PackManifestVersion1_1_RC4, MediaTypeFliptFeatures, oras.PackManifestOptions{ + ManifestAnnotations: map[string]string{}, + Layers: layers, + }) + if err != nil { + return Bundle{}, nil + } + + if s.reference.Reference != "" { + if err := store.Tag(ctx, desc, s.reference.Reference); err != nil { + return Bundle{}, nil + } + } + + bundle := Bundle{ + Digest: desc.Digest, + Repository: s.reference.Repository, + Tag: s.reference.Reference, + } + + bundle.CreatedAt, err = parseCreated(desc.Annotations) + if err != nil { + return Bundle{}, nil + } + + return bundle, nil +} + +func (s *Store) buildLayers(ctx context.Context, store oras.Target, src fs.FS) (layers []v1.Descriptor, _ error) { + if err := storagefs.WalkDocuments(s.logger, src, func(doc *ext.Document) error { + fmt.Println(doc) + payload, err := json.Marshal(&doc) + if err != nil { + return err + } + + desc := v1.Descriptor{ + Digest: digest.FromBytes(payload), + Size: int64(len(payload)), + MediaType: MediaTypeFliptNamespace, + Annotations: map[string]string{ + AnnotationFliptNamespace: doc.Namespace, + }, + } + + s.logger.Debug("adding layer", zap.String("digest", desc.Digest.Hex()), zap.String("namespace", doc.Namespace)) + + if err := store.Push(ctx, desc, bytes.NewReader(payload)); err != nil && !errors.Is(err, errdef.ErrAlreadyExists) { + return err + } + + layers = append(layers, desc) + return nil + }); err != nil { + return nil, err + } + return layers, nil +} + func getMediaTypeAndEncoding(layer v1.Descriptor) (mediaType, encoding string, _ error) { var ok bool if mediaType = layer.MediaType; mediaType == "" { @@ -317,6 +418,10 @@ func (f FileInfo) Sys() any { return nil } +func parseCreated(annotations map[string]string) (time.Time, error) { + return time.Parse(time.RFC3339, annotations[v1.AnnotationCreated]) +} + func defaultBundleDirectory() (string, error) { dir, err := config.Dir() if err != nil { diff --git a/internal/oci/file_test.go b/internal/oci/file_test.go index 47447d0069..55a665c6f3 100644 --- a/internal/oci/file_test.go +++ b/internal/oci/file_test.go @@ -3,8 +3,10 @@ package oci import ( "bytes" "context" + "embed" "fmt" "io" + "io/fs" "path" "testing" @@ -12,23 +14,24 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" "oras.land/oras-go/v2" "oras.land/oras-go/v2/content/oci" ) func TestNewStore(t *testing.T) { t.Run("unexpected scheme", func(t *testing.T) { - _, err := NewStore("fake://local/something:latest") + _, err := NewStore(zaptest.NewLogger(t), "fake://local/something:latest") require.EqualError(t, err, `unexpected repository scheme: "fake" should be one of [http|https|flipt]`) }) t.Run("invalid reference", func(t *testing.T) { - _, err := NewStore("something:latest") + _, err := NewStore(zaptest.NewLogger(t), "something:latest") require.EqualError(t, err, `invalid reference: missing repository`) }) t.Run("invalid local reference", func(t *testing.T) { - _, err := NewStore("flipt://invalid/something:latest") + _, err := NewStore(zaptest.NewLogger(t), "flipt://invalid/something:latest") require.EqualError(t, err, `unexpected local reference: "invalid/something:latest"`) }) @@ -40,7 +43,7 @@ func TestNewStore(t *testing.T) { "https://remote/something:latest", } { t.Run(repository, func(t *testing.T) { - _, err := NewStore(repository, WithBundleDir(t.TempDir())) + _, err := NewStore(zaptest.NewLogger(t), repository, WithBundleDir(t.TempDir())) require.NoError(t, err) }) } @@ -53,7 +56,7 @@ func TestStore_Fetch_InvalidMediaType(t *testing.T) { ) repository := fmt.Sprintf("flipt://local/%s:latest", repo) - store, err := NewStore(repository, WithBundleDir(dir)) + store, err := NewStore(zaptest.NewLogger(t), repository, WithBundleDir(dir)) require.NoError(t, err) ctx := context.Background() @@ -64,7 +67,7 @@ func TestStore_Fetch_InvalidMediaType(t *testing.T) { layer("default", `{"namespace":"default"}`, MediaTypeFliptNamespace+"+unknown"), ) - store, err = NewStore(repository, WithBundleDir(dir)) + store, err = NewStore(zaptest.NewLogger(t), repository, WithBundleDir(dir)) require.NoError(t, err) _, err = store.Fetch(ctx) @@ -77,7 +80,7 @@ func TestStore_Fetch(t *testing.T) { layer("other", `namespace: other`, MediaTypeFliptNamespace+"+yaml"), ) - store, err := NewStore(fmt.Sprintf("flipt://local/%s:latest", repo), WithBundleDir(dir)) + store, err := NewStore(zaptest.NewLogger(t), fmt.Sprintf("flipt://local/%s:latest", repo), WithBundleDir(dir)) require.NoError(t, err) ctx := context.Background() @@ -121,6 +124,34 @@ func TestStore_Fetch(t *testing.T) { }) } +//go:embed testdata/* +var testdata embed.FS + +func TestStore_Build(t *testing.T) { + ctx := context.TODO() + dir, repo := testRepository(t) + + store, err := NewStore(zaptest.NewLogger(t), fmt.Sprintf("flipt://local/%s:latest", repo), WithBundleDir(dir)) + require.NoError(t, err) + + testdata, err := fs.Sub(testdata, "testdata") + require.NoError(t, err) + + bundle, err := store.Build(ctx, testdata) + require.NoError(t, err) + + assert.Equal(t, repo, bundle.Repository) + assert.Equal(t, "latest", bundle.Tag) + assert.NotEmpty(t, bundle.Digest) + assert.NotEmpty(t, bundle.CreatedAt) + + resp, err := store.Fetch(ctx) + require.NoError(t, err) + require.False(t, resp.Matched) + + assert.Len(t, resp.Files, 1) +} + func layer(ns, payload, mediaType string) func(*testing.T, oras.Target) v1.Descriptor { return func(t *testing.T, store oras.Target) v1.Descriptor { t.Helper() @@ -155,6 +186,10 @@ func testRepository(t *testing.T, layerFuncs ...func(*testing.T, oras.Target) v1 ctx := context.TODO() + if len(layerFuncs) == 0 { + return + } + var layers []v1.Descriptor for _, fn := range layerFuncs { layers = append(layers, fn(t, store)) diff --git a/internal/oci/testdata/.flipt.yml b/internal/oci/testdata/.flipt.yml new file mode 100644 index 0000000000..4880a0638c --- /dev/null +++ b/internal/oci/testdata/.flipt.yml @@ -0,0 +1,3 @@ +version: 1.0 +include: + - production.yml diff --git a/internal/oci/testdata/production.yml b/internal/oci/testdata/production.yml new file mode 100644 index 0000000000..600bcc0406 --- /dev/null +++ b/internal/oci/testdata/production.yml @@ -0,0 +1 @@ +namespace: production diff --git a/internal/storage/fs/oci/source.go b/internal/storage/fs/oci/source.go index dcd0f38d10..3e5906ca12 100644 --- a/internal/storage/fs/oci/source.go +++ b/internal/storage/fs/oci/source.go @@ -59,7 +59,7 @@ func (s *Source) Get(context.Context) (*storagefs.StoreSnapshot, error) { return s.curSnap, nil } - if s.curSnap, err = storagefs.SnapshotFromFiles(resp.Files...); err != nil { + if s.curSnap, err = storagefs.SnapshotFromFiles(s.logger, resp.Files...); err != nil { return nil, err } diff --git a/internal/storage/fs/oci/source_test.go b/internal/storage/fs/oci/source_test.go index c68bb2aee0..9d72c5c975 100644 --- a/internal/storage/fs/oci/source_test.go +++ b/internal/storage/fs/oci/source_test.go @@ -91,7 +91,7 @@ func testSource(t *testing.T) (*Source, oras.Target) { layer("production", `{"namespace":"production"}`, fliptoci.MediaTypeFliptNamespace), ) - store, err := fliptoci.NewStore(fmt.Sprintf("flipt://local/%s:latest", repo), fliptoci.WithBundleDir(dir)) + store, err := fliptoci.NewStore(zaptest.NewLogger(t), fmt.Sprintf("flipt://local/%s:latest", repo), fliptoci.WithBundleDir(dir)) require.NoError(t, err) source, err := NewSource(zaptest.NewLogger(t), diff --git a/internal/storage/fs/snapshot.go b/internal/storage/fs/snapshot.go index 02586176e6..1efb0cfd5c 100644 --- a/internal/storage/fs/snapshot.go +++ b/internal/storage/fs/snapshot.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "io/fs" + "path/filepath" "sort" "strconv" @@ -87,12 +88,12 @@ func SnapshotFromFS(logger *zap.Logger, fs fs.FS) (*StoreSnapshot, error) { logger.Debug("opening state files", zap.Strings("paths", files)) - return SnapshotFromPaths(fs, files...) + return SnapshotFromPaths(logger, fs, files...) } // SnapshotFromPaths constructs a StoreSnapshot from the provided // slice of paths resolved against the provided fs.FS. -func SnapshotFromPaths(ffs fs.FS, paths ...string) (*StoreSnapshot, error) { +func SnapshotFromPaths(logger *zap.Logger, ffs fs.FS, paths ...string) (*StoreSnapshot, error) { var files []fs.File for _, file := range paths { fi, err := ffs.Open(file) @@ -103,17 +104,12 @@ func SnapshotFromPaths(ffs fs.FS, paths ...string) (*StoreSnapshot, error) { files = append(files, fi) } - return SnapshotFromFiles(files...) + return SnapshotFromFiles(logger, files...) } // SnapshotFromFiles constructs a StoreSnapshot from the provided slice // of fs.File implementations. -func SnapshotFromFiles(files ...fs.File) (*StoreSnapshot, error) { - validator, err := cue.NewFeaturesValidator() - if err != nil { - return nil, err - } - +func SnapshotFromFiles(logger *zap.Logger, files ...fs.File) (*StoreSnapshot, error) { now := timestamppb.Now() s := StoreSnapshot{ ns: map[string]*namespace{ @@ -124,52 +120,109 @@ func SnapshotFromFiles(files ...fs.File) (*StoreSnapshot, error) { } for _, fi := range files { - if err := func() error { - defer fi.Close() + defer fi.Close() - stat, err := fi.Stat() - if err != nil { - return err - } + doc, err := documentFromFile(logger, fi) + if err != nil { + return nil, err + } - buf := &bytes.Buffer{} - reader := io.TeeReader(fi, buf) + if err := s.addDoc(doc); err != nil { + return nil, err + } + } - if err := validator.Validate(stat.Name(), reader); err != nil { - return err - } + return &s, nil +} - decoder := yaml.NewDecoder(buf) - // Support YAML stream by looping until we reach an EOF. - for { - doc := new(ext.Document) +// WalkDocuments walks all the Flipt feature documents found in the target fs.FS +// based on either the default index file or an index file located in the root +func WalkDocuments(logger *zap.Logger, src fs.FS, fn func(*ext.Document) error) error { + paths, err := listStateFiles(logger, src) + if err != nil { + return err + } - if err := decoder.Decode(doc); err != nil { - if errors.Is(err, io.EOF) { - break - } - return err - } + for _, file := range paths { + logger.Debug("opening state file", zap.String("path", file)) - // set namespace to default if empty in document - if doc.Namespace == "" { - doc.Namespace = "default" - } + fi, err := src.Open(file) + if err != nil { + return err + } + defer fi.Close() - if err := s.addDoc(doc); err != nil { - return err + doc, err := documentFromFile(logger, fi) + if err != nil { + return err + } + + if err := fn(doc); err != nil { + return err + } + } + + return nil +} + +// documentFromFile parses and validates a document from a single fs.File instance +func documentFromFile(logger *zap.Logger, fi fs.File) (*ext.Document, error) { + validator, err := cue.NewFeaturesValidator() + if err != nil { + return nil, err + } + + stat, err := fi.Stat() + if err != nil { + return nil, err + } + + buf := &bytes.Buffer{} + reader := io.TeeReader(fi, buf) + + if err := validator.Validate(stat.Name(), reader); err != nil { + return nil, err + } + + extn := filepath.Ext(stat.Name()) + switch extn { + case ".yaml", ".yml": + decoder := yaml.NewDecoder(buf) + // Support YAML stream by looping until we reach an EOF. + for { + doc := &ext.Document{} + if err := decoder.Decode(doc); err != nil { + if errors.Is(err, io.EOF) { + break } + return nil, err } - return nil - }(); err != nil { + // set namespace to default if empty in document + if doc.Namespace == "" { + doc.Namespace = "default" + } + + return doc, nil + } + case "", ".json": + doc := &ext.Document{} + if err := json.NewDecoder(buf).Decode(&doc); err != nil { return nil, err } + + // set namespace to default if empty in document + if doc.Namespace == "" { + doc.Namespace = "default" + } + + return doc, nil } - return &s, nil + return nil, fmt.Errorf("unexpected extension: %q", extn) } +// listStateFiles lists all the file paths in a provided fs.FS containing Flipt feature state func listStateFiles(logger *zap.Logger, source fs.FS) ([]string, error) { // This is the default variable + value for the FliptIndex. It will preserve its value if // a .flipt.yml can not be read for whatever reason. From cd7992913f3f69c4dba45fa07c366bf535493f97 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Fri, 3 Nov 2023 17:42:24 +0000 Subject: [PATCH 04/20] fix(cmd/bundle): correct typo in help message --- build/testing/testdata/cli.txt | 1 + cmd/flipt/bundle.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/build/testing/testdata/cli.txt b/build/testing/testdata/cli.txt index 56c0918769..84bc052f8d 100644 --- a/build/testing/testdata/cli.txt +++ b/build/testing/testdata/cli.txt @@ -11,6 +11,7 @@ $ flipt --config /path/to/config.yml migrate Available Commands: + bundle Manage Flipt bundles config Manage Flipt configuration export Export Flipt data to file/stdout help Help about any command diff --git a/cmd/flipt/bundle.go b/cmd/flipt/bundle.go index 326a2f931d..a747520633 100644 --- a/cmd/flipt/bundle.go +++ b/cmd/flipt/bundle.go @@ -18,12 +18,12 @@ func newBundleCommand() *cobra.Command { cmd := &cobra.Command{ Use: "bundle", - Short: "Manage flipt bundlees", + Short: "Manage Flipt bundles", } cmd.AddCommand(&cobra.Command{ Use: "build", - Short: "build a bundle", + Short: "Build a bundle", RunE: bundle.build, Args: cobra.ExactArgs(1), }) From b6248a2a2a1ac2f5c9c196dd8256ae6e7e2f61a1 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Fri, 3 Nov 2023 17:54:47 +0000 Subject: [PATCH 05/20] fix(oci): walk entire stream of documents --- cmd/flipt/bundle.go | 4 +- internal/ext/common.go | 121 +++++++++++++++++++++----------- internal/oci/file.go | 10 +-- internal/oci/file_test.go | 17 ++--- internal/storage/fs/snapshot.go | 57 ++++++++------- 5 files changed, 122 insertions(+), 87 deletions(-) diff --git a/cmd/flipt/bundle.go b/cmd/flipt/bundle.go index a747520633..6d2755a37f 100644 --- a/cmd/flipt/bundle.go +++ b/cmd/flipt/bundle.go @@ -9,9 +9,7 @@ import ( "go.flipt.io/flipt/internal/oci" ) -type bundleCommand struct { - rootDir string -} +type bundleCommand struct{} func newBundleCommand() *cobra.Command { bundle := &bundleCommand{} diff --git a/internal/ext/common.go b/internal/ext/common.go index 3473c9b370..373471959f 100644 --- a/internal/ext/common.go +++ b/internal/ext/common.go @@ -1,77 +1,78 @@ package ext import ( + "encoding/json" "errors" ) type Document struct { - Version string `yaml:"version,omitempty"` - Namespace string `yaml:"namespace,omitempty"` - Flags []*Flag `yaml:"flags,omitempty"` - Segments []*Segment `yaml:"segments,omitempty"` + Version string `yaml:"version,omitempty" json:"version,omitempty"` + Namespace string `yaml:"namespace,omitempty" json:"namespace,omitempty"` + Flags []*Flag `yaml:"flags,omitempty" json:"flags,omitempty"` + Segments []*Segment `yaml:"segments,omitempty" json:"segments,omitempty"` } type Flag struct { - Key string `yaml:"key,omitempty"` - Name string `yaml:"name,omitempty"` - Type string `yaml:"type,omitempty"` - Description string `yaml:"description,omitempty"` - Enabled bool `yaml:"enabled"` - Variants []*Variant `yaml:"variants,omitempty"` - Rules []*Rule `yaml:"rules,omitempty"` - Rollouts []*Rollout `yaml:"rollouts,omitempty"` + Key string `yaml:"key,omitempty" json:"key,omitempty"` + Name string `yaml:"name,omitempty" json:"name,omitempty"` + Type string `yaml:"type,omitempty" json:"type,omitempty"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + Enabled bool `yaml:"enabled" json:"enabled,omitempty"` + Variants []*Variant `yaml:"variants,omitempty" json:"variants,omitempty"` + Rules []*Rule `yaml:"rules,omitempty" json:"rules,omitempty"` + Rollouts []*Rollout `yaml:"rollouts,omitempty" json:"rollouts,omitempty"` } type Variant struct { - Key string `yaml:"key,omitempty"` - Name string `yaml:"name,omitempty"` - Description string `yaml:"description,omitempty"` - Attachment interface{} `yaml:"attachment,omitempty"` + Key string `yaml:"key,omitempty" json:"key,omitempty"` + Name string `yaml:"name,omitempty" json:"name,omitempty"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + Attachment interface{} `yaml:"attachment,omitempty" json:"attachment,omitempty"` } type Rule struct { - Segment *SegmentEmbed `yaml:"segment,omitempty"` - Rank uint `yaml:"rank,omitempty"` - Distributions []*Distribution `yaml:"distributions,omitempty"` + Segment *SegmentEmbed `yaml:"segment,omitempty" json:"segment,omitempty"` + Rank uint `yaml:"rank,omitempty" json:"rank,omitempty"` + Distributions []*Distribution `yaml:"distributions,omitempty" json:"distributions,omitempty"` } type Distribution struct { - VariantKey string `yaml:"variant,omitempty"` - Rollout float32 `yaml:"rollout,omitempty"` + VariantKey string `yaml:"variant,omitempty" json:"variant,omitempty"` + Rollout float32 `yaml:"rollout,omitempty" json:"rollout,omitempty"` } type Rollout struct { - Description string `yaml:"description,omitempty"` - Segment *SegmentRule `yaml:"segment,omitempty"` - Threshold *ThresholdRule `yaml:"threshold,omitempty"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + Segment *SegmentRule `yaml:"segment,omitempty" json:"segment,omitempty"` + Threshold *ThresholdRule `yaml:"threshold,omitempty" json:"threshold,omitempty"` } type SegmentRule struct { - Key string `yaml:"key,omitempty"` - Keys []string `yaml:"keys,omitempty"` - Operator string `yaml:"operator,omitempty"` - Value bool `yaml:"value,omitempty"` + Key string `yaml:"key,omitempty" json:"key,omitempty"` + Keys []string `yaml:"keys,omitempty" json:"keys,omitempty"` + Operator string `yaml:"operator,omitempty" json:"operator,omitempty"` + Value bool `yaml:"value,omitempty" json:"value,omitempty"` } type ThresholdRule struct { - Percentage float32 `yaml:"percentage,omitempty"` - Value bool `yaml:"value,omitempty"` + Percentage float32 `yaml:"percentage,omitempty" json:"percentage,omitempty"` + Value bool `yaml:"value,omitempty" json:"value,omitempty"` } type Segment struct { - Key string `yaml:"key,omitempty"` - Name string `yaml:"name,omitempty"` - Description string `yaml:"description,omitempty"` - Constraints []*Constraint `yaml:"constraints,omitempty"` - MatchType string `yaml:"match_type,omitempty"` + Key string `yaml:"key,omitempty" json:"key,omitempty"` + Name string `yaml:"name,omitempty" json:"name,omitempty"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + Constraints []*Constraint `yaml:"constraints,omitempty" json:"constraints,omitempty"` + MatchType string `yaml:"match_type,omitempty" json:"match_type,omitempty"` } type Constraint struct { - Type string `yaml:"type,omitempty"` - Property string `yaml:"property,omitempty"` - Operator string `yaml:"operator,omitempty"` - Value string `yaml:"value,omitempty"` - Description string `yaml:"description,omitempty"` + Type string `yaml:"type,omitempty" json:"type,omitempty"` + Property string `yaml:"property,omitempty" json:"property,omitempty"` + Operator string `yaml:"operator,omitempty" json:"operator,omitempty"` + Value string `yaml:"value,omitempty" json:"value,omitempty"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` } type SegmentEmbed struct { @@ -114,6 +115,42 @@ func (s *SegmentEmbed) UnmarshalYAML(unmarshal func(interface{}) error) error { return errors.New("failed to unmarshal to string or segmentKeys") } +// MarshalJSON tries to type assert to either of the following types that implement +// IsSegment, and returns the marshaled value. +func (s *SegmentEmbed) MarshalJSON() ([]byte, error) { + switch t := s.IsSegment.(type) { + case SegmentKey: + return json.Marshal(string(t)) + case *Segments: + sk := &Segments{ + Keys: t.Keys, + SegmentOperator: t.SegmentOperator, + } + return json.Marshal(sk) + } + + return nil, errors.New("failed to marshal to string or segmentKeys") +} + +// UnmarshalJSON attempts to unmarshal a string or `SegmentKeys`, and fails if it can not +// do so. +func (s *SegmentEmbed) UnmarshalJSON(v []byte) error { + var sk SegmentKey + + if err := json.Unmarshal(v, &sk); err == nil { + s.IsSegment = sk + return nil + } + + var sks *Segments + if err := json.Unmarshal(v, &sks); err == nil { + s.IsSegment = sks + return nil + } + + return errors.New("failed to unmarshal to string or segmentKeys") +} + // IsSegment is used to unify the two types of segments that can come in // from the import. type IsSegment interface { @@ -125,8 +162,8 @@ type SegmentKey string func (s SegmentKey) IsSegment() {} type Segments struct { - Keys []string `yaml:"keys,omitempty"` - SegmentOperator string `yaml:"operator,omitempty"` + Keys []string `yaml:"keys,omitempty" json:"keys,omitempty"` + SegmentOperator string `yaml:"operator,omitempty" json:"operator,omitempty"` } func (s *Segments) IsSegment() {} diff --git a/internal/oci/file.go b/internal/oci/file.go index f74b5a763c..a86f49f7f1 100644 --- a/internal/oci/file.go +++ b/internal/oci/file.go @@ -285,12 +285,12 @@ type Bundle struct { func (s *Store) Build(ctx context.Context, src fs.FS) (Bundle, error) { store, err := s.store() if err != nil { - return Bundle{}, nil + return Bundle{}, err } layers, err := s.buildLayers(ctx, store, src) if err != nil { - return Bundle{}, nil + return Bundle{}, err } desc, err := oras.PackManifest(ctx, store, oras.PackManifestVersion1_1_RC4, MediaTypeFliptFeatures, oras.PackManifestOptions{ @@ -298,12 +298,12 @@ func (s *Store) Build(ctx context.Context, src fs.FS) (Bundle, error) { Layers: layers, }) if err != nil { - return Bundle{}, nil + return Bundle{}, err } if s.reference.Reference != "" { if err := store.Tag(ctx, desc, s.reference.Reference); err != nil { - return Bundle{}, nil + return Bundle{}, err } } @@ -315,7 +315,7 @@ func (s *Store) Build(ctx context.Context, src fs.FS) (Bundle, error) { bundle.CreatedAt, err = parseCreated(desc.Annotations) if err != nil { - return Bundle{}, nil + return Bundle{}, err } return bundle, nil diff --git a/internal/oci/file_test.go b/internal/oci/file_test.go index 55a665c6f3..717b24b61a 100644 --- a/internal/oci/file_test.go +++ b/internal/oci/file_test.go @@ -19,6 +19,8 @@ import ( "oras.land/oras-go/v2/content/oci" ) +const repo = "testrepo" + func TestNewStore(t *testing.T) { t.Run("unexpected scheme", func(t *testing.T) { _, err := NewStore(zaptest.NewLogger(t), "fake://local/something:latest") @@ -51,7 +53,7 @@ func TestNewStore(t *testing.T) { } func TestStore_Fetch_InvalidMediaType(t *testing.T) { - dir, repo := testRepository(t, + dir := testRepository(t, layer("default", `{"namespace":"default"}`, "unexpected.media.type"), ) @@ -63,7 +65,7 @@ func TestStore_Fetch_InvalidMediaType(t *testing.T) { _, err = store.Fetch(ctx) require.EqualError(t, err, "layer \"sha256:85ee577ad99c62f314abca9f43ad87c2ee8818513e6383a77690df56d0352748\": type \"unexpected.media.type\": unexpected media type") - dir, repo = testRepository(t, + dir = testRepository(t, layer("default", `{"namespace":"default"}`, MediaTypeFliptNamespace+"+unknown"), ) @@ -75,7 +77,7 @@ func TestStore_Fetch_InvalidMediaType(t *testing.T) { } func TestStore_Fetch(t *testing.T) { - dir, repo := testRepository(t, + dir := testRepository(t, layer("default", `{"namespace":"default"}`, MediaTypeFliptNamespace), layer("other", `namespace: other`, MediaTypeFliptNamespace+"+yaml"), ) @@ -129,7 +131,7 @@ var testdata embed.FS func TestStore_Build(t *testing.T) { ctx := context.TODO() - dir, repo := testRepository(t) + dir := testRepository(t) store, err := NewStore(zaptest.NewLogger(t), fmt.Sprintf("flipt://local/%s:latest", repo), WithBundleDir(dir)) require.NoError(t, err) @@ -171,15 +173,14 @@ func layer(ns, payload, mediaType string) func(*testing.T, oras.Target) v1.Descr } } -func testRepository(t *testing.T, layerFuncs ...func(*testing.T, oras.Target) v1.Descriptor) (dir, repository string) { +func testRepository(t *testing.T, layerFuncs ...func(*testing.T, oras.Target) v1.Descriptor) (dir string) { t.Helper() - repository = "testrepo" dir = t.TempDir() - t.Log("test OCI directory", dir, repository) + t.Log("test OCI directory", dir, repo) - store, err := oci.New(path.Join(dir, repository)) + store, err := oci.New(path.Join(dir, repo)) require.NoError(t, err) store.AutoSaveIndex = true diff --git a/internal/storage/fs/snapshot.go b/internal/storage/fs/snapshot.go index 1efb0cfd5c..f3b98fa72c 100644 --- a/internal/storage/fs/snapshot.go +++ b/internal/storage/fs/snapshot.go @@ -122,13 +122,15 @@ func SnapshotFromFiles(logger *zap.Logger, files ...fs.File) (*StoreSnapshot, er for _, fi := range files { defer fi.Close() - doc, err := documentFromFile(logger, fi) + docs, err := documentsFromFile(fi) if err != nil { return nil, err } - if err := s.addDoc(doc); err != nil { - return nil, err + for _, doc := range docs { + if err := s.addDoc(doc); err != nil { + return nil, err + } } } @@ -152,21 +154,23 @@ func WalkDocuments(logger *zap.Logger, src fs.FS, fn func(*ext.Document) error) } defer fi.Close() - doc, err := documentFromFile(logger, fi) + docs, err := documentsFromFile(fi) if err != nil { return err } - if err := fn(doc); err != nil { - return err + for _, doc := range docs { + if err := fn(doc); err != nil { + return err + } } } return nil } -// documentFromFile parses and validates a document from a single fs.File instance -func documentFromFile(logger *zap.Logger, fi fs.File) (*ext.Document, error) { +// documentsFromFile parses and validates a document from a single fs.File instance +func documentsFromFile(fi fs.File) ([]*ext.Document, error) { validator, err := cue.NewFeaturesValidator() if err != nil { return nil, err @@ -184,30 +188,26 @@ func documentFromFile(logger *zap.Logger, fi fs.File) (*ext.Document, error) { return nil, err } + var docs []*ext.Document extn := filepath.Ext(stat.Name()) + + var decode func(any) error switch extn { case ".yaml", ".yml": - decoder := yaml.NewDecoder(buf) // Support YAML stream by looping until we reach an EOF. - for { - doc := &ext.Document{} - if err := decoder.Decode(doc); err != nil { - if errors.Is(err, io.EOF) { - break - } - return nil, err - } - - // set namespace to default if empty in document - if doc.Namespace == "" { - doc.Namespace = "default" - } - - return doc, nil - } + decode = yaml.NewDecoder(buf).Decode case "", ".json": + decode = json.NewDecoder(buf).Decode + default: + return nil, fmt.Errorf("unexpected extension: %q", extn) + } + + for { doc := &ext.Document{} - if err := json.NewDecoder(buf).Decode(&doc); err != nil { + if err := decode(doc); err != nil { + if errors.Is(err, io.EOF) { + break + } return nil, err } @@ -215,11 +215,10 @@ func documentFromFile(logger *zap.Logger, fi fs.File) (*ext.Document, error) { if doc.Namespace == "" { doc.Namespace = "default" } - - return doc, nil + docs = append(docs, doc) } - return nil, fmt.Errorf("unexpected extension: %q", extn) + return docs, nil } // listStateFiles lists all the file paths in a provided fs.FS containing Flipt feature state From 706d9caf18b76ef6c1aab7055d558172a68a570f Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Tue, 7 Nov 2023 12:58:14 +0000 Subject: [PATCH 06/20] chore(oci): remove debug println --- internal/oci/file.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/oci/file.go b/internal/oci/file.go index a86f49f7f1..3f9da40cad 100644 --- a/internal/oci/file.go +++ b/internal/oci/file.go @@ -323,7 +323,6 @@ func (s *Store) Build(ctx context.Context, src fs.FS) (Bundle, error) { func (s *Store) buildLayers(ctx context.Context, store oras.Target, src fs.FS) (layers []v1.Descriptor, _ error) { if err := storagefs.WalkDocuments(s.logger, src, func(doc *ext.Document) error { - fmt.Println(doc) payload, err := json.Marshal(&doc) if err != nil { return err From 9bbe22a4b8a4492b0c5b9978800545adae2c863e Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Tue, 7 Nov 2023 13:08:25 +0000 Subject: [PATCH 07/20] test(oci): add more testdata to build case --- internal/oci/file_test.go | 2 +- internal/oci/testdata/.flipt.yml | 1 + internal/oci/testdata/default.yml | 25 +++++++++++++++++++++++++ internal/oci/testdata/production.yml | 25 +++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 internal/oci/testdata/default.yml diff --git a/internal/oci/file_test.go b/internal/oci/file_test.go index 717b24b61a..b57ebc8db4 100644 --- a/internal/oci/file_test.go +++ b/internal/oci/file_test.go @@ -151,7 +151,7 @@ func TestStore_Build(t *testing.T) { require.NoError(t, err) require.False(t, resp.Matched) - assert.Len(t, resp.Files, 1) + assert.Len(t, resp.Files, 2) } func layer(ns, payload, mediaType string) func(*testing.T, oras.Target) v1.Descriptor { diff --git a/internal/oci/testdata/.flipt.yml b/internal/oci/testdata/.flipt.yml index 4880a0638c..c2103e459b 100644 --- a/internal/oci/testdata/.flipt.yml +++ b/internal/oci/testdata/.flipt.yml @@ -1,3 +1,4 @@ version: 1.0 include: + - default.yml - production.yml diff --git a/internal/oci/testdata/default.yml b/internal/oci/testdata/default.yml new file mode 100644 index 0000000000..bf4ce6fa6a --- /dev/null +++ b/internal/oci/testdata/default.yml @@ -0,0 +1,25 @@ +flags: +- key: flag_boolean + name: FLAG_BOOLEAN + type: BOOLEAN_FLAG_TYPE + description: Boolean Flag Description + enabled: false + rollouts: + - description: enabled for segment_001 + segment: + key: segment_001 + value: true + - description: disabled for segment_002 + segment: + key: segment_002 + - description: enabled for segment_003 + segment: + key: segment_003 + value: true + - description: disabled for segment_004 + segment: + key: segment_004 + - description: enabled for 50% + threshold: + percentage: 50 + value: true diff --git a/internal/oci/testdata/production.yml b/internal/oci/testdata/production.yml index 600bcc0406..ae481e0548 100644 --- a/internal/oci/testdata/production.yml +++ b/internal/oci/testdata/production.yml @@ -1 +1,26 @@ namespace: production +flags: +- key: flag_boolean + name: FLAG_BOOLEAN + type: BOOLEAN_FLAG_TYPE + description: Boolean Flag Description + enabled: false + rollouts: + - description: enabled for segment_001 + segment: + key: segment_001 + value: true + - description: disabled for segment_002 + segment: + key: segment_002 + - description: enabled for segment_003 + segment: + key: segment_003 + value: true + - description: disabled for segment_004 + segment: + key: segment_004 + - description: enabled for 50% + threshold: + percentage: 50 + value: true From 195695521119096e1340e3219c83dfd5ceda6322 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Tue, 7 Nov 2023 16:42:19 +0000 Subject: [PATCH 08/20] feat(cmd/flipt): implement bundle list --- cmd/flipt/bundle.go | 67 +++++++++++--- internal/oci/file.go | 174 ++++++++++++++++++++++++---------- internal/oci/file_test.go | 190 ++++++++++++++++++++++++++++++-------- 3 files changed, 329 insertions(+), 102 deletions(-) diff --git a/cmd/flipt/bundle.go b/cmd/flipt/bundle.go index 6d2755a37f..c307f708c0 100644 --- a/cmd/flipt/bundle.go +++ b/cmd/flipt/bundle.go @@ -3,6 +3,7 @@ package main import ( "fmt" "os" + "text/tabwriter" "github.com/spf13/cobra" "go.flipt.io/flipt/internal/containers" @@ -26,15 +27,63 @@ func newBundleCommand() *cobra.Command { Args: cobra.ExactArgs(1), }) + cmd.AddCommand(&cobra.Command{ + Use: "list", + Short: "List all bundles", + RunE: bundle.list, + }) + return cmd } func (c *bundleCommand) build(cmd *cobra.Command, args []string) error { - logger, cfg, err := buildConfig() + store, err := c.getStore() + if err != nil { + return err + } + + ref, err := oci.ParseReference(args[0]) + if err != nil { + return err + } + + bundle, err := store.Build(cmd.Context(), os.DirFS("."), ref) if err != nil { return err } + fmt.Println(bundle.Digest) + + return nil +} + +func (c *bundleCommand) list(cmd *cobra.Command, args []string) error { + store, err := c.getStore() + if err != nil { + return err + } + + bundles, err := store.List(cmd.Context()) + if err != nil { + return err + } + + wr := writer() + + fmt.Fprintf(wr, "DIGEST\tREPO\tTAG\tCREATED\t\n") + for _, bundle := range bundles { + fmt.Fprintf(wr, "%s\t%s\t%s\t%s\t\n", bundle.Digest.Hex()[:7], bundle.Repository, bundle.Tag, bundle.CreatedAt) + } + + return wr.Flush() +} + +func (c *bundleCommand) getStore() (*oci.Store, error) { + logger, cfg, err := buildConfig() + if err != nil { + return nil, err + } + var opts []containers.Option[oci.StoreOptions] if cfg := cfg.Storage.OCI; cfg != nil { if cfg.BundleDirectory != "" { @@ -49,17 +98,9 @@ func (c *bundleCommand) build(cmd *cobra.Command, args []string) error { } } - store, err := oci.NewStore(logger, fmt.Sprintf("flipt://local/%s", args[0]), opts...) - if err != nil { - return err - } - - bundle, err := store.Build(cmd.Context(), os.DirFS(".")) - if err != nil { - return err - } - - fmt.Println(bundle.Digest) + return oci.NewStore(logger, opts...) +} - return nil +func writer() *tabwriter.Writer { + return tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0) } diff --git a/internal/oci/file.go b/internal/oci/file.go index 3f9da40cad..00a282f0dc 100644 --- a/internal/oci/file.go +++ b/internal/oci/file.go @@ -33,10 +33,9 @@ import ( // Store is a type which can retrieve Flipt feature files from a target repository and reference // Repositories can be local (OCI layout directories on the filesystem) or a remote registry type Store struct { - logger *zap.Logger - reference registry.Reference - store func() (oras.Target, error) - local oras.Target + opts StoreOptions + logger *zap.Logger + local oras.Target } // StoreOptions are used to configure call to NewStore @@ -73,15 +72,31 @@ func WithCredentials(user, pass string) containers.Option[StoreOptions] { } // NewStore constructs and configures an instance of *Store for the provided config -func NewStore(logger *zap.Logger, repository string, opts ...containers.Option[StoreOptions]) (*Store, error) { +func NewStore(logger *zap.Logger, opts ...containers.Option[StoreOptions]) (*Store, error) { + store := &Store{ + opts: StoreOptions{}, + logger: logger, + local: memory.New(), + } + dir, err := defaultBundleDirectory() if err != nil { return nil, err } - options := StoreOptions{bundleDir: dir} - containers.ApplyAll(&options, opts...) + store.opts.bundleDir = dir + + containers.ApplyAll(&store.opts, opts...) + + return store, nil +} + +type Reference struct { + registry.Reference + Scheme string +} +func ParseReference(repository string) (Reference, error) { scheme, repository, match := strings.Cut(repository, "://") // support empty scheme as remote and https if !match { @@ -89,61 +104,62 @@ func NewStore(logger *zap.Logger, repository string, opts ...containers.Option[S scheme = "https" } + if !strings.Contains(repository, "/") { + repository = "local/" + repository + scheme = "flipt" + } + ref, err := registry.ParseReference(repository) if err != nil { - return nil, err + return Reference{}, nil } - store := &Store{ - logger: logger, - reference: ref, - local: memory.New(), - } switch scheme { + case "http", "https": + case "flipt": + if ref.Registry != "local" { + return Reference{}, fmt.Errorf("unexpected local reference: %q", ref) + } + default: + return Reference{}, fmt.Errorf("unexpected repository scheme: %q should be one of [http|https|flipt]", scheme) + } + + return Reference{ + Reference: ref, + Scheme: scheme, + }, nil +} + +func (s *Store) getTarget(ctx context.Context, ref Reference) (oras.Target, error) { + switch ref.Scheme { case "http", "https": remote, err := remote.NewRepository(fmt.Sprintf("%s/%s", ref.Registry, ref.Repository)) if err != nil { return nil, err } - remote.PlainHTTP = scheme == "http" + remote.PlainHTTP = ref.Scheme == "http" - store.store = func() (oras.Target, error) { - return remote, nil - } + return remote, nil case "flipt": - if ref.Registry != "local" { - return nil, fmt.Errorf("unexpected local reference: %q", ref) - } - // build the store once to ensure it is valid - bundleDir := path.Join(options.bundleDir, ref.Repository) + bundleDir := path.Join(s.opts.bundleDir, ref.Repository) _, err := oci.New(bundleDir) if err != nil { return nil, err } - store.store = func() (oras.Target, error) { - // we recreate the OCI store on every operation for local - // because the oci library we use maintains a reference cache - // in memory that doesn't get purged when the target directory - // contents changes underneath it - // this allows us to change the state with another process and - // have the store pickup the changes - store, err := oci.New(bundleDir) - if err != nil { - return nil, err - } + store, err := oci.New(bundleDir) + if err != nil { + return nil, err + } - store.AutoSaveIndex = true + store.AutoSaveIndex = true - return store, nil - } - default: - return nil, fmt.Errorf("unexpected repository scheme: %q should be one of [http|https|flipt]", scheme) + return store, nil } - return store, nil + return nil, fmt.Errorf("unexpected repository scheme: %q should be one of [http|https|flipt]", ref.Scheme) } // FetchOptions configures a call to Fetch @@ -172,20 +188,20 @@ func IfNoMatch(digest digest.Digest) containers.Option[FetchOptions] { // Fetch retrieves the associated files for the tracked repository and reference // It can optionally be configured to skip fetching given the caller has a digest // that matches the current reference target -func (s *Store) Fetch(ctx context.Context, opts ...containers.Option[FetchOptions]) (*FetchResponse, error) { +func (s *Store) Fetch(ctx context.Context, ref Reference, opts ...containers.Option[FetchOptions]) (*FetchResponse, error) { var options FetchOptions containers.ApplyAll(&options, opts...) - store, err := s.store() + store, err := s.getTarget(ctx, ref) if err != nil { return nil, err } desc, err := oras.Copy(ctx, store, - s.reference.Reference, + ref.Reference.Reference, s.local, - s.reference.Reference, + ref.Reference.Reference, oras.DefaultCopyOptions) if err != nil { return nil, err @@ -280,10 +296,70 @@ type Bundle struct { CreatedAt time.Time } +// List returns a slice of bundles available on the host +func (s *Store) List(ctx context.Context) (bundles []Bundle, _ error) { + fi, err := os.Open(s.opts.bundleDir) + if err != nil { + return nil, err + } + + defer fi.Close() + + entries, err := fi.ReadDir(-1) + if err != nil { + return nil, err + } + + for _, entry := range entries { + bytes, err := os.ReadFile(filepath.Join(s.opts.bundleDir, entry.Name(), v1.ImageIndexFile)) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + + return nil, err + } + + var index v1.Index + if err := json.Unmarshal(bytes, &index); err != nil { + return nil, err + } + + for _, manifest := range index.Manifests { + digest := manifest.Digest + path := filepath.Join(s.opts.bundleDir, entry.Name(), "blobs", digest.Algorithm().String(), digest.Hex()) + bytes, err := os.ReadFile(path) + if err != nil { + return nil, err + } + + var man v1.Manifest + if err := json.Unmarshal(bytes, &man); err != nil { + return nil, err + } + + bundle := Bundle{ + Digest: manifest.Digest, + Repository: entry.Name(), + Tag: manifest.Annotations[v1.AnnotationRefName], + } + + bundle.CreatedAt, err = parseCreated(man.Annotations) + if err != nil { + return nil, err + } + + bundles = append(bundles, bundle) + } + } + + return +} + // Build bundles the target directory Flipt feature state into the target configured on the Store // It returns a Bundle which contains metadata regarding the resulting bundle details -func (s *Store) Build(ctx context.Context, src fs.FS) (Bundle, error) { - store, err := s.store() +func (s *Store) Build(ctx context.Context, src fs.FS, ref Reference) (Bundle, error) { + store, err := s.getTarget(ctx, ref) if err != nil { return Bundle{}, err } @@ -301,16 +377,16 @@ func (s *Store) Build(ctx context.Context, src fs.FS) (Bundle, error) { return Bundle{}, err } - if s.reference.Reference != "" { - if err := store.Tag(ctx, desc, s.reference.Reference); err != nil { + if ref.Reference.Reference != "" { + if err := store.Tag(ctx, desc, ref.Reference.Reference); err != nil { return Bundle{}, err } } bundle := Bundle{ Digest: desc.Digest, - Repository: s.reference.Repository, - Tag: s.reference.Reference, + Repository: ref.Repository, + Tag: ref.Reference.Reference, } bundle.CreatedAt, err = parseCreated(desc.Annotations) diff --git a/internal/oci/file_test.go b/internal/oci/file_test.go index b57ebc8db4..e46cb30f5d 100644 --- a/internal/oci/file_test.go +++ b/internal/oci/file_test.go @@ -4,11 +4,13 @@ import ( "bytes" "context" "embed" + "errors" "fmt" "io" "io/fs" "path" "testing" + "time" "github.com/opencontainers/go-digest" v1 "github.com/opencontainers/image-spec/specs-go/v1" @@ -17,39 +19,100 @@ import ( "go.uber.org/zap/zaptest" "oras.land/oras-go/v2" "oras.land/oras-go/v2/content/oci" + "oras.land/oras-go/v2/registry" ) const repo = "testrepo" -func TestNewStore(t *testing.T) { - t.Run("unexpected scheme", func(t *testing.T) { - _, err := NewStore(zaptest.NewLogger(t), "fake://local/something:latest") - require.EqualError(t, err, `unexpected repository scheme: "fake" should be one of [http|https|flipt]`) - }) - - t.Run("invalid reference", func(t *testing.T) { - _, err := NewStore(zaptest.NewLogger(t), "something:latest") - require.EqualError(t, err, `invalid reference: missing repository`) - }) - - t.Run("invalid local reference", func(t *testing.T) { - _, err := NewStore(zaptest.NewLogger(t), "flipt://invalid/something:latest") - require.EqualError(t, err, `unexpected local reference: "invalid/something:latest"`) - }) - - t.Run("valid", func(t *testing.T) { - for _, repository := range []string{ - "flipt://local/something:latest", - "remote/something:latest", - "http://remote/something:latest", - "https://remote/something:latest", - } { - t.Run(repository, func(t *testing.T) { - _, err := NewStore(zaptest.NewLogger(t), repository, WithBundleDir(t.TempDir())) - require.NoError(t, err) - }) - } - }) +func TestParseReference(t *testing.T) { + for _, test := range []struct { + name string + reference string + expected Reference + expectedErr error + }{ + { + name: "unexpected scheme", + reference: "fake://local/something:latest", + expectedErr: errors.New(`unexpected repository scheme: "fake" should be one of [http|https|flipt]`), + }, + { + name: "invalid local reference", + reference: "flipt://invalid/something:latest", + expectedErr: errors.New(`unexpected local reference: "invalid/something:latest"`), + }, + { + name: "valid local", + reference: "flipt://local/something:latest", + expected: Reference{ + Reference: registry.Reference{ + Registry: "local", + Repository: "something", + Reference: "latest", + }, + Scheme: "flipt", + }, + }, + { + name: "valid bare local", + reference: "something:latest", + expected: Reference{ + Reference: registry.Reference{ + Registry: "local", + Repository: "something", + Reference: "latest", + }, + Scheme: "flipt", + }, + }, + { + name: "valid insecure remote", + reference: "http://remote/something:latest", + expected: Reference{ + Reference: registry.Reference{ + Registry: "remote", + Repository: "something", + Reference: "latest", + }, + Scheme: "http", + }, + }, + { + name: "valid remote", + reference: "https://remote/something:latest", + expected: Reference{ + Reference: registry.Reference{ + Registry: "remote", + Repository: "something", + Reference: "latest", + }, + Scheme: "https", + }, + }, + { + name: "valid bare remote", + reference: "remote/something:latest", + expected: Reference{ + Reference: registry.Reference{ + Registry: "remote", + Repository: "something", + Reference: "latest", + }, + Scheme: "https", + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + ref, err := ParseReference(test.reference) + if test.expectedErr != nil { + require.Equal(t, test.expectedErr, err) + return + } + + require.Nil(t, err) + assert.Equal(t, test.expected, ref) + }) + } } func TestStore_Fetch_InvalidMediaType(t *testing.T) { @@ -57,22 +120,24 @@ func TestStore_Fetch_InvalidMediaType(t *testing.T) { layer("default", `{"namespace":"default"}`, "unexpected.media.type"), ) - repository := fmt.Sprintf("flipt://local/%s:latest", repo) - store, err := NewStore(zaptest.NewLogger(t), repository, WithBundleDir(dir)) + ref, err := ParseReference(fmt.Sprintf("flipt://local/%s:latest", repo)) + require.NoError(t, err) + + store, err := NewStore(zaptest.NewLogger(t), WithBundleDir(dir)) require.NoError(t, err) ctx := context.Background() - _, err = store.Fetch(ctx) + _, err = store.Fetch(ctx, ref) require.EqualError(t, err, "layer \"sha256:85ee577ad99c62f314abca9f43ad87c2ee8818513e6383a77690df56d0352748\": type \"unexpected.media.type\": unexpected media type") dir = testRepository(t, layer("default", `{"namespace":"default"}`, MediaTypeFliptNamespace+"+unknown"), ) - store, err = NewStore(zaptest.NewLogger(t), repository, WithBundleDir(dir)) + store, err = NewStore(zaptest.NewLogger(t), WithBundleDir(dir)) require.NoError(t, err) - _, err = store.Fetch(ctx) + _, err = store.Fetch(ctx, ref) require.EqualError(t, err, "layer \"sha256:85ee577ad99c62f314abca9f43ad87c2ee8818513e6383a77690df56d0352748\": unexpected layer encoding: \"unknown\"") } @@ -82,11 +147,14 @@ func TestStore_Fetch(t *testing.T) { layer("other", `namespace: other`, MediaTypeFliptNamespace+"+yaml"), ) - store, err := NewStore(zaptest.NewLogger(t), fmt.Sprintf("flipt://local/%s:latest", repo), WithBundleDir(dir)) + ref, err := ParseReference(fmt.Sprintf("flipt://local/%s:latest", repo)) + require.NoError(t, err) + + store, err := NewStore(zaptest.NewLogger(t), WithBundleDir(dir)) require.NoError(t, err) ctx := context.Background() - resp, err := store.Fetch(ctx) + resp, err := store.Fetch(ctx, ref) require.NoError(t, err) require.False(t, resp.Matched, "matched an empty digest unexpectedly") @@ -117,7 +185,7 @@ func TestStore_Fetch(t *testing.T) { assert.Equal(t, expected, found) t.Run("IfNoMatch", func(t *testing.T) { - resp, err = store.Fetch(ctx, IfNoMatch(manifestDigest)) + resp, err = store.Fetch(ctx, ref, IfNoMatch(manifestDigest)) require.NoError(t, err) require.True(t, resp.Matched) @@ -133,13 +201,16 @@ func TestStore_Build(t *testing.T) { ctx := context.TODO() dir := testRepository(t) - store, err := NewStore(zaptest.NewLogger(t), fmt.Sprintf("flipt://local/%s:latest", repo), WithBundleDir(dir)) + ref, err := ParseReference(fmt.Sprintf("flipt://local/%s:latest", repo)) + require.NoError(t, err) + + store, err := NewStore(zaptest.NewLogger(t), WithBundleDir(dir)) require.NoError(t, err) testdata, err := fs.Sub(testdata, "testdata") require.NoError(t, err) - bundle, err := store.Build(ctx, testdata) + bundle, err := store.Build(ctx, testdata, ref) require.NoError(t, err) assert.Equal(t, repo, bundle.Repository) @@ -147,13 +218,52 @@ func TestStore_Build(t *testing.T) { assert.NotEmpty(t, bundle.Digest) assert.NotEmpty(t, bundle.CreatedAt) - resp, err := store.Fetch(ctx) + resp, err := store.Fetch(ctx, ref) require.NoError(t, err) require.False(t, resp.Matched) assert.Len(t, resp.Files, 2) } +func TestStore_List(t *testing.T) { + ctx := context.TODO() + dir := testRepository(t) + + ref, err := ParseReference(fmt.Sprintf("%s:latest", repo)) + require.NoError(t, err) + + store, err := NewStore(zaptest.NewLogger(t), WithBundleDir(dir)) + require.NoError(t, err) + + bundles, err := store.List(ctx) + require.NoError(t, err) + require.Len(t, bundles, 0) + + testdata, err := fs.Sub(testdata, "testdata") + require.NoError(t, err) + + bundle, err := store.Build(ctx, testdata, ref) + require.NoError(t, err) + + t.Log("bundle created digest:", bundle.Digest) + + // sleep long enough for 1 second to pass + // to bump the timestamp on next build + time.Sleep(1 * time.Second) + + bundle, err = store.Build(ctx, testdata, ref) + require.NoError(t, err) + + t.Log("bundle created digest:", bundle.Digest) + + bundles, err = store.List(ctx) + require.NoError(t, err) + require.Len(t, bundles, 2) + + assert.Equal(t, "latest", bundles[0].Tag) + assert.Empty(t, bundles[1].Tag) +} + func layer(ns, payload, mediaType string) func(*testing.T, oras.Target) v1.Descriptor { return func(t *testing.T, store oras.Target) v1.Descriptor { t.Helper() From 1abc22109ce0bbbb232ff5fa15f37dcaa332b614 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Tue, 7 Nov 2023 18:17:36 +0000 Subject: [PATCH 09/20] feat(cmd): add JSON support to import/export --- cmd/flipt/export.go | 15 +- cmd/flipt/import.go | 10 +- internal/ext/common.go | 2 +- internal/ext/encoding.go | 57 ++++++++ internal/ext/exporter.go | 10 +- internal/ext/exporter_test.go | 50 +++++-- internal/ext/importer.go | 5 +- internal/ext/importer_fuzz_test.go | 2 +- internal/ext/importer_test.go | 131 ++++++++++-------- internal/ext/testdata/export.json | 1 + .../ext/testdata/export_all_namespaces.json | 3 + .../ext/testdata/export_all_namespaces.yml | 2 - .../ext/testdata/export_default_and_foo.json | 2 + .../ext/testdata/export_default_and_foo.yml | 1 - internal/ext/testdata/import.json | 1 + .../testdata/import_implicit_rule_rank.json | 1 + .../ext/testdata/import_invalid_version.json | 1 + .../ext/testdata/import_no_attachment.json | 1 + .../import_rule_multiple_segments.json | 1 + .../testdata/import_single_namespace_foo.json | 1 + ...import_two_namespaces_default_and_foo.json | 2 + internal/ext/testdata/import_v1.json | 1 + internal/ext/testdata/import_v1_1.json | 1 + .../import_v1_flag_type_not_supported.json | 1 + .../import_v1_rollouts_not_supported.json | 1 + ...ort_yaml_stream_all_unique_namespaces.json | 3 + .../import_yaml_stream_default_namespace.json | 2 + 27 files changed, 216 insertions(+), 92 deletions(-) create mode 100644 internal/ext/encoding.go create mode 100644 internal/ext/testdata/export.json create mode 100644 internal/ext/testdata/export_all_namespaces.json create mode 100644 internal/ext/testdata/export_default_and_foo.json create mode 100644 internal/ext/testdata/import.json create mode 100644 internal/ext/testdata/import_implicit_rule_rank.json create mode 100644 internal/ext/testdata/import_invalid_version.json create mode 100644 internal/ext/testdata/import_no_attachment.json create mode 100644 internal/ext/testdata/import_rule_multiple_segments.json create mode 100644 internal/ext/testdata/import_single_namespace_foo.json create mode 100644 internal/ext/testdata/import_two_namespaces_default_and_foo.json create mode 100644 internal/ext/testdata/import_v1.json create mode 100644 internal/ext/testdata/import_v1_1.json create mode 100644 internal/ext/testdata/import_v1_flag_type_not_supported.json create mode 100644 internal/ext/testdata/import_v1_rollouts_not_supported.json create mode 100644 internal/ext/testdata/import_yaml_stream_all_unique_namespaces.json create mode 100644 internal/ext/testdata/import_yaml_stream_default_namespace.json diff --git a/cmd/flipt/export.go b/cmd/flipt/export.go index 822857572c..997ea7ea0b 100644 --- a/cmd/flipt/export.go +++ b/cmd/flipt/export.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "path/filepath" "time" "github.com/spf13/cobra" @@ -87,6 +88,7 @@ func (c *exportCommand) run(cmd *cobra.Command, _ []string) error { // default to stdout out io.Writer = os.Stdout logger = zap.Must(zap.NewDevelopment()) + enc = ext.EncodingYML ) // export to file @@ -103,6 +105,11 @@ func (c *exportCommand) run(cmd *cobra.Command, _ []string) error { fmt.Fprintf(fi, "# exported by Flipt (%s) on %s\n\n", version, time.Now().UTC().Format(time.RFC3339)) out = fi + + if extn := filepath.Ext(c.filename); len(extn) > 0 { + // strip off the leading . + enc = ext.Encoding(extn[1:]) + } } // Use client when remote address is configured. @@ -111,7 +118,7 @@ func (c *exportCommand) run(cmd *cobra.Command, _ []string) error { if err != nil { return err } - return c.export(cmd.Context(), out, client) + return c.export(cmd.Context(), enc, out, client) } // Otherwise, go direct to the DB using Flipt configuration file. @@ -131,9 +138,9 @@ func (c *exportCommand) run(cmd *cobra.Command, _ []string) error { defer cleanup() - return c.export(cmd.Context(), out, server) + return c.export(cmd.Context(), enc, out, server) } -func (c *exportCommand) export(ctx context.Context, dst io.Writer, lister ext.Lister) error { - return ext.NewExporter(lister, c.namespaces, c.allNamespaces).Export(ctx, dst) +func (c *exportCommand) export(ctx context.Context, enc ext.Encoding, dst io.Writer, lister ext.Lister) error { + return ext.NewExporter(lister, c.namespaces, c.allNamespaces).Export(ctx, enc, dst) } diff --git a/cmd/flipt/import.go b/cmd/flipt/import.go index 68dc16cfb6..025062d990 100644 --- a/cmd/flipt/import.go +++ b/cmd/flipt/import.go @@ -65,6 +65,7 @@ func (c *importCommand) run(cmd *cobra.Command, args []string) error { var ( in io.Reader = os.Stdin logger = zap.Must(zap.NewDevelopment()) + enc = ext.EncodingYML ) if !c.importStdin { @@ -89,6 +90,11 @@ func (c *importCommand) run(cmd *cobra.Command, args []string) error { defer fi.Close() in = fi + + if extn := filepath.Ext(importFilename); len(extn) > 0 { + // strip off leading . + enc = ext.Encoding(extn[1:]) + } } // Use client when remote address is configured. @@ -97,7 +103,7 @@ func (c *importCommand) run(cmd *cobra.Command, args []string) error { if err != nil { return err } - return ext.NewImporter(client).Import(cmd.Context(), in) + return ext.NewImporter(client).Import(cmd.Context(), enc, in) } logger, cfg, err := buildConfig() @@ -150,5 +156,5 @@ func (c *importCommand) run(cmd *cobra.Command, args []string) error { return ext.NewImporter( server, - ).Import(cmd.Context(), in) + ).Import(cmd.Context(), enc, in) } diff --git a/internal/ext/common.go b/internal/ext/common.go index 373471959f..ce8ea23872 100644 --- a/internal/ext/common.go +++ b/internal/ext/common.go @@ -17,7 +17,7 @@ type Flag struct { Name string `yaml:"name,omitempty" json:"name,omitempty"` Type string `yaml:"type,omitempty" json:"type,omitempty"` Description string `yaml:"description,omitempty" json:"description,omitempty"` - Enabled bool `yaml:"enabled" json:"enabled,omitempty"` + Enabled bool `yaml:"enabled" json:"enabled"` Variants []*Variant `yaml:"variants,omitempty" json:"variants,omitempty"` Rules []*Rule `yaml:"rules,omitempty" json:"rules,omitempty"` Rollouts []*Rollout `yaml:"rollouts,omitempty" json:"rollouts,omitempty"` diff --git a/internal/ext/encoding.go b/internal/ext/encoding.go new file mode 100644 index 0000000000..12d4fbce62 --- /dev/null +++ b/internal/ext/encoding.go @@ -0,0 +1,57 @@ +package ext + +import ( + "encoding/json" + "io" + + "gopkg.in/yaml.v2" +) + +type Encoding string + +const ( + EncodingYML Encoding = "yml" + EncodingYAML Encoding = "yaml" + EncodingJSON Encoding = "json" +) + +func (e Encoding) NewEncoder(w io.Writer) EncodeCloser { + switch e { + case EncodingYML, EncodingYAML: + return yaml.NewEncoder(w) + case EncodingJSON: + return NopCloseEncoder{json.NewEncoder(w)} + } + + return nil +} + +type Encoder interface { + Encode(any) error +} + +type EncodeCloser interface { + Encoder + Close() error +} + +type NopCloseEncoder struct { + Encoder +} + +func (n NopCloseEncoder) Close() error { return nil } + +func (e Encoding) NewDecoder(r io.Reader) Decoder { + switch e { + case EncodingYML, EncodingYAML: + return yaml.NewDecoder(r) + case EncodingJSON: + return json.NewDecoder(r) + } + + return nil +} + +type Decoder interface { + Decode(any) error +} diff --git a/internal/ext/exporter.go b/internal/ext/exporter.go index 46d5df3015..be0680bf14 100644 --- a/internal/ext/exporter.go +++ b/internal/ext/exporter.go @@ -9,7 +9,6 @@ import ( "github.com/blang/semver/v4" "go.flipt.io/flipt/rpc/flipt" - "gopkg.in/yaml.v2" ) const ( @@ -56,9 +55,9 @@ func versionString(v semver.Version) string { return fmt.Sprintf("%d.%d", v.Major, v.Minor) } -func (e *Exporter) Export(ctx context.Context, w io.Writer) error { +func (e *Exporter) Export(ctx context.Context, encoding Encoding, w io.Writer) error { var ( - enc = yaml.NewEncoder(w) + enc = encoding.NewEncoder(w) batchSize = e.batchSize ) @@ -97,8 +96,7 @@ func (e *Exporter) Export(ctx context.Context, w io.Writer) error { for i := 0; i < len(namespaces); i++ { doc := new(Document) - // Only provide the version to the first document in the YAML - // file. + // Only provide the version to the first document in the stream. if i == 0 { doc.Version = versionString(latestVersion) } @@ -285,8 +283,6 @@ func (e *Exporter) Export(ctx context.Context, w io.Writer) error { } } - // The YAML encoder does the stream separation by default, so no need to write to the file the - // "---" separator manually. if err := enc.Encode(doc); err != nil { return fmt.Errorf("marshaling document: %w", err) } diff --git a/internal/ext/exporter_test.go b/internal/ext/exporter_test.go index 9662e7d6ab..41028d13e8 100644 --- a/internal/ext/exporter_test.go +++ b/internal/ext/exporter_test.go @@ -3,10 +3,13 @@ package ext import ( "bytes" "context" + "fmt" + "io" "os" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.flipt.io/flipt/rpc/flipt" ) @@ -208,7 +211,7 @@ func TestExport(t *testing.T) { }, }, }, - path: "testdata/export.yml", + path: "testdata/export", namespaces: "default", allNamespaces: false, }, @@ -469,7 +472,7 @@ func TestExport(t *testing.T) { }, }, }, - path: "testdata/export_default_and_foo.yml", + path: "testdata/export_default_and_foo", namespaces: "default,foo", allNamespaces: false, }, @@ -741,24 +744,47 @@ func TestExport(t *testing.T) { }, }, }, - path: "testdata/export_all_namespaces.yml", + path: "testdata/export_all_namespaces", namespaces: "", allNamespaces: true, }, } for _, tc := range tests { - var ( - exporter = NewExporter(tc.lister, tc.namespaces, tc.allNamespaces) - b = new(bytes.Buffer) - ) + tc := tc + for _, ext := range extensions { + t.Run(fmt.Sprintf("%s (%s)", tc.name, ext), func(t *testing.T) { + var ( + exporter = NewExporter(tc.lister, tc.namespaces, tc.allNamespaces) + b = new(bytes.Buffer) + ) - err := exporter.Export(context.Background(), b) - assert.NoError(t, err) + err := exporter.Export(context.Background(), ext, b) + assert.NoError(t, err) - in, err := os.ReadFile(tc.path) - assert.NoError(t, err) + in, err := os.ReadFile(tc.path + "." + string(ext)) + assert.NoError(t, err) - assert.YAMLEq(t, string(in), b.String()) + var ( + expected = ext.NewDecoder(bytes.NewReader(in)) + found = ext.NewDecoder(b) + ) + + // handle newline delimeted JSON + for { + var exp, fnd any + eerr := expected.Decode(&exp) + ferr := found.Decode(&fnd) + require.Equal(t, eerr, ferr) + + if ferr == io.EOF { + break + } + require.NoError(t, ferr) + + assert.Equal(t, exp, fnd) + } + }) + } } } diff --git a/internal/ext/importer.go b/internal/ext/importer.go index 2d6f832cbf..afde0ccece 100644 --- a/internal/ext/importer.go +++ b/internal/ext/importer.go @@ -12,7 +12,6 @@ import ( "go.flipt.io/flipt/rpc/flipt" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "gopkg.in/yaml.v2" ) type Creator interface { @@ -45,9 +44,9 @@ func NewImporter(store Creator, opts ...ImportOpt) *Importer { return i } -func (i *Importer) Import(ctx context.Context, r io.Reader) (err error) { +func (i *Importer) Import(ctx context.Context, enc Encoding, r io.Reader) (err error) { var ( - dec = yaml.NewDecoder(r) + dec = enc.NewDecoder(r) version semver.Version ) diff --git a/internal/ext/importer_fuzz_test.go b/internal/ext/importer_fuzz_test.go index 53cbe56645..a1bc305ba1 100644 --- a/internal/ext/importer_fuzz_test.go +++ b/internal/ext/importer_fuzz_test.go @@ -20,7 +20,7 @@ func FuzzImport(f *testing.F) { f.Fuzz(func(t *testing.T, in []byte) { importer := NewImporter(&mockCreator{}) - if err := importer.Import(context.Background(), bytes.NewReader(in)); err != nil { + if err := importer.Import(context.Background(), EncodingYAML, bytes.NewReader(in)); err != nil { // we only care about panics t.Skip() } diff --git a/internal/ext/importer_test.go b/internal/ext/importer_test.go index 08ae054579..054412a3ec 100644 --- a/internal/ext/importer_test.go +++ b/internal/ext/importer_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "os" "testing" @@ -12,6 +13,8 @@ import ( "go.flipt.io/flipt/rpc/flipt" ) +var extensions = []Encoding{EncodingYML, EncodingJSON} + type mockCreator struct { getNSReqs []*flipt.GetNamespaceRequest getNSErr error @@ -187,7 +190,7 @@ func TestImport(t *testing.T) { }{ { name: "import with attachment", - path: "testdata/import.yml", + path: "testdata/import", expected: &mockCreator{ flagReqs: []*flipt.CreateFlagRequest{ { @@ -274,7 +277,7 @@ func TestImport(t *testing.T) { }, { name: "import without attachment", - path: "testdata/import_no_attachment.yml", + path: "testdata/import_no_attachment", expected: &mockCreator{ flagReqs: []*flipt.CreateFlagRequest{ { @@ -360,7 +363,7 @@ func TestImport(t *testing.T) { }, { name: "import with implicit rule ranks", - path: "testdata/import_implicit_rule_rank.yml", + path: "testdata/import_implicit_rule_rank", expected: &mockCreator{ flagReqs: []*flipt.CreateFlagRequest{ { @@ -447,7 +450,7 @@ func TestImport(t *testing.T) { }, { name: "import with multiple segments", - path: "testdata/import_rule_multiple_segments.yml", + path: "testdata/import_rule_multiple_segments", expected: &mockCreator{ flagReqs: []*flipt.CreateFlagRequest{ { @@ -534,7 +537,7 @@ func TestImport(t *testing.T) { }, { name: "import v1", - path: "testdata/import_v1.yml", + path: "testdata/import_v1", expected: &mockCreator{ flagReqs: []*flipt.CreateFlagRequest{ { @@ -590,7 +593,7 @@ func TestImport(t *testing.T) { }, { name: "import v1.1", - path: "testdata/import_v1_1.yml", + path: "testdata/import_v1_1", expected: &mockCreator{ flagReqs: []*flipt.CreateFlagRequest{ { @@ -679,21 +682,23 @@ func TestImport(t *testing.T) { for _, tc := range tests { tc := tc - t.Run(tc.name, func(t *testing.T) { - var ( - creator = &mockCreator{} - importer = NewImporter(creator) - ) - - in, err := os.Open(tc.path) - assert.NoError(t, err) - defer in.Close() - - err = importer.Import(context.Background(), in) - assert.NoError(t, err) - - assert.Equal(t, tc.expected, creator) - }) + for _, ext := range extensions { + t.Run(fmt.Sprintf("%s (%s)", tc.name, ext), func(t *testing.T) { + var ( + creator = &mockCreator{} + importer = NewImporter(creator) + ) + + in, err := os.Open(tc.path + "." + string(ext)) + assert.NoError(t, err) + defer in.Close() + + err = importer.Import(context.Background(), Encoding(ext), in) + assert.NoError(t, err) + + assert.Equal(t, tc.expected, creator) + }) + } } } @@ -707,7 +712,7 @@ func TestImport_Export(t *testing.T) { assert.NoError(t, err) defer in.Close() - err = importer.Import(context.Background(), in) + err = importer.Import(context.Background(), EncodingYML, in) require.NoError(t, err) assert.Equal(t, "default", creator.flagReqs[0].NamespaceKey) } @@ -718,12 +723,14 @@ func TestImport_InvalidVersion(t *testing.T) { importer = NewImporter(creator) ) - in, err := os.Open("testdata/import_invalid_version.yml") - assert.NoError(t, err) - defer in.Close() + for _, ext := range extensions { + in, err := os.Open("testdata/import_invalid_version." + string(ext)) + assert.NoError(t, err) + defer in.Close() - err = importer.Import(context.Background(), in) - assert.EqualError(t, err, "unsupported version: 5.0") + err = importer.Import(context.Background(), Encoding(ext), in) + assert.EqualError(t, err, "unsupported version: 5.0") + } } func TestImport_FlagType_LTVersion1_1(t *testing.T) { @@ -732,12 +739,14 @@ func TestImport_FlagType_LTVersion1_1(t *testing.T) { importer = NewImporter(creator) ) - in, err := os.Open("testdata/import_v1_flag_type_not_supported.yml") - assert.NoError(t, err) - defer in.Close() + for _, ext := range extensions { + in, err := os.Open("testdata/import_v1_flag_type_not_supported." + string(ext)) + assert.NoError(t, err) + defer in.Close() - err = importer.Import(context.Background(), in) - assert.EqualError(t, err, "flag.type is supported in version >=1.1, found 1.0") + err = importer.Import(context.Background(), Encoding(ext), in) + assert.EqualError(t, err, "flag.type is supported in version >=1.1, found 1.0") + } } func TestImport_Rollouts_LTVersion1_1(t *testing.T) { @@ -746,12 +755,14 @@ func TestImport_Rollouts_LTVersion1_1(t *testing.T) { importer = NewImporter(creator) ) - in, err := os.Open("testdata/import_v1_rollouts_not_supported.yml") - assert.NoError(t, err) - defer in.Close() + for _, ext := range extensions { + in, err := os.Open("testdata/import_v1_rollouts_not_supported." + string(ext)) + assert.NoError(t, err) + defer in.Close() - err = importer.Import(context.Background(), in) - assert.EqualError(t, err, "flag.rollouts is supported in version >=1.1, found 1.0") + err = importer.Import(context.Background(), Encoding(ext), in) + assert.EqualError(t, err, "flag.rollouts is supported in version >=1.1, found 1.0") + } } func TestImport_Namespaces_Mix_And_Match(t *testing.T) { @@ -764,35 +775,35 @@ func TestImport_Namespaces_Mix_And_Match(t *testing.T) { }{ { name: "single namespace no YAML stream", - path: "testdata/import.yml", + path: "testdata/import", expectedGetNSReqs: 0, expectedCreateFlagReqs: 2, expectedCreateSegmentReqs: 1, }, { name: "single namespace foo non-YAML stream", - path: "testdata/import_single_namespace_foo.yml", + path: "testdata/import_single_namespace_foo", expectedGetNSReqs: 1, expectedCreateFlagReqs: 2, expectedCreateSegmentReqs: 1, }, { name: "multiple namespaces default and foo", - path: "testdata/import_two_namespaces_default_and_foo.yml", + path: "testdata/import_two_namespaces_default_and_foo", expectedGetNSReqs: 1, expectedCreateFlagReqs: 4, expectedCreateSegmentReqs: 2, }, { name: "yaml stream only default namespace", - path: "testdata/import_yaml_stream_default_namespace.yml", + path: "testdata/import_yaml_stream_default_namespace", expectedGetNSReqs: 0, expectedCreateFlagReqs: 4, expectedCreateSegmentReqs: 2, }, { name: "yaml stream all unqiue namespaces", - path: "testdata/import_yaml_stream_all_unique_namespaces.yml", + path: "testdata/import_yaml_stream_all_unique_namespaces", expectedGetNSReqs: 2, expectedCreateFlagReqs: 6, expectedCreateSegmentReqs: 3, @@ -801,23 +812,25 @@ func TestImport_Namespaces_Mix_And_Match(t *testing.T) { for _, tc := range tests { tc := tc - t.Run(tc.name, func(t *testing.T) { - var ( - creator = &mockCreator{} - importer = NewImporter(creator) - ) - - in, err := os.Open(tc.path) - assert.NoError(t, err) - defer in.Close() - - err = importer.Import(context.Background(), in) - assert.NoError(t, err) - - assert.Len(t, creator.getNSReqs, tc.expectedGetNSReqs) - assert.Len(t, creator.flagReqs, tc.expectedCreateFlagReqs) - assert.Len(t, creator.segmentReqs, tc.expectedCreateSegmentReqs) - }) + for _, ext := range extensions { + t.Run(fmt.Sprintf("%s (%s)", tc.name, ext), func(t *testing.T) { + var ( + creator = &mockCreator{} + importer = NewImporter(creator) + ) + + in, err := os.Open(tc.path + "." + string(ext)) + assert.NoError(t, err) + defer in.Close() + + err = importer.Import(context.Background(), Encoding(ext), in) + assert.NoError(t, err) + + assert.Len(t, creator.getNSReqs, tc.expectedGetNSReqs) + assert.Len(t, creator.flagReqs, tc.expectedCreateFlagReqs) + assert.Len(t, creator.segmentReqs, tc.expectedCreateSegmentReqs) + }) + } } } diff --git a/internal/ext/testdata/export.json b/internal/ext/testdata/export.json new file mode 100644 index 0000000000..e5101f9055 --- /dev/null +++ b/internal/ext/testdata/export.json @@ -0,0 +1 @@ +{"version":"1.2","namespace":"default","flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","attachment":{"pi":3.141,"happy":true,"name":"Niels","nothing":null,"answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}},{"key":"foo"}],"rules":[{"segment":"segment1","distributions":[{"variant":"variant1","rollout":100}]},{"segment":{"keys":["segment1","segment2"],"operator":"AND_SEGMENT_OPERATOR"}}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"foo","operator":"eq","value":"baz","description":"desc"},{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz","description":"desc"}]},{"key":"segment2","name":"segment2","match_type":"ANY_MATCH_TYPE","description":"description"}]} diff --git a/internal/ext/testdata/export_all_namespaces.json b/internal/ext/testdata/export_all_namespaces.json new file mode 100644 index 0000000000..40dbd9667b --- /dev/null +++ b/internal/ext/testdata/export_all_namespaces.json @@ -0,0 +1,3 @@ +{"version":"1.2","namespace":"default"} +{"namespace":"foo","flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","attachment":{"pi":3.141,"happy":true,"name":"Niels","nothing":null,"answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}},{"key":"foo"}],"rules":[{"segment":"segment1","distributions":[{"variant":"variant1","rollout":100}]},{"segment":{"keys":["segment1","segment2"],"operator":"AND_SEGMENT_OPERATOR"}}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"foo","operator":"eq","value":"baz","description":"desc"},{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz","description":"desc"}]},{"key":"segment2","name":"segment2","match_type":"ANY_MATCH_TYPE","description":"description"}]} +{"namespace":"bar","flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","attachment":{"pi":3.141,"happy":true,"name":"Niels","nothing":null,"answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}},{"key":"foo"}],"rules":[{"segment":"segment1","distributions":[{"variant":"variant1","rollout":100}]},{"segment":{"keys":["segment1","segment2"],"operator":"AND_SEGMENT_OPERATOR"}}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"foo","operator":"eq","value":"baz","description":"desc"},{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz","description":"desc"}]},{"key":"segment2","name":"segment2","match_type":"ANY_MATCH_TYPE","description":"description"}]} diff --git a/internal/ext/testdata/export_all_namespaces.yml b/internal/ext/testdata/export_all_namespaces.yml index 6896deb33a..635c07be56 100644 --- a/internal/ext/testdata/export_all_namespaces.yml +++ b/internal/ext/testdata/export_all_namespaces.yml @@ -1,7 +1,6 @@ version: "1.2" namespace: default --- -version: "1.2" namespace: foo flags: - key: flag1 @@ -72,7 +71,6 @@ segments: match_type: "ANY_MATCH_TYPE" description: description --- -version: "1.2" namespace: bar flags: - key: flag1 diff --git a/internal/ext/testdata/export_default_and_foo.json b/internal/ext/testdata/export_default_and_foo.json new file mode 100644 index 0000000000..3963ba84c1 --- /dev/null +++ b/internal/ext/testdata/export_default_and_foo.json @@ -0,0 +1,2 @@ +{"version":"1.2","namespace":"default","flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","attachment":{"pi":3.141,"happy":true,"name":"Niels","nothing":null,"answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}},{"key":"foo"}],"rules":[{"segment":"segment1","distributions":[{"variant":"variant1","rollout":100}]},{"segment":{"keys":["segment1","segment2"],"operator":"AND_SEGMENT_OPERATOR"}}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"foo","operator":"eq","value":"baz","description":"desc"},{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz","description":"desc"}]},{"key":"segment2","name":"segment2","match_type":"ANY_MATCH_TYPE","description":"description"}]} +{"namespace":"foo","flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","attachment":{"pi":3.141,"happy":true,"name":"Niels","nothing":null,"answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}},{"key":"foo"}],"rules":[{"segment":"segment1","distributions":[{"variant":"variant1","rollout":100}]},{"segment":{"keys":["segment1","segment2"],"operator":"AND_SEGMENT_OPERATOR"}}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"foo","operator":"eq","value":"baz","description":"desc"},{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz","description":"desc"}]},{"key":"segment2","name":"segment2","match_type":"ANY_MATCH_TYPE","description":"description"}]} diff --git a/internal/ext/testdata/export_default_and_foo.yml b/internal/ext/testdata/export_default_and_foo.yml index 3e1e9232a1..abe67ae136 100644 --- a/internal/ext/testdata/export_default_and_foo.yml +++ b/internal/ext/testdata/export_default_and_foo.yml @@ -69,7 +69,6 @@ segments: match_type: "ANY_MATCH_TYPE" description: description --- -version: "1.2" namespace: foo flags: - key: flag1 diff --git a/internal/ext/testdata/import.json b/internal/ext/testdata/import.json new file mode 100644 index 0000000000..8f71dd37b4 --- /dev/null +++ b/internal/ext/testdata/import.json @@ -0,0 +1 @@ +{"flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","description":"variant description","attachment":{"pi":3.141,"happy":true,"name":"Niels","answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}}],"rules":[{"segment":"segment1","rank":1,"distributions":[{"variant":"variant1","rollout":100}]}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz"}]}]} diff --git a/internal/ext/testdata/import_implicit_rule_rank.json b/internal/ext/testdata/import_implicit_rule_rank.json new file mode 100644 index 0000000000..662867b540 --- /dev/null +++ b/internal/ext/testdata/import_implicit_rule_rank.json @@ -0,0 +1 @@ +{"flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","description":"variant description","attachment":{"pi":3.141,"happy":true,"name":"Niels","answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}}],"rules":[{"segment":"segment1","distributions":[{"variant":"variant1","rollout":100}]}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz"}]}]} diff --git a/internal/ext/testdata/import_invalid_version.json b/internal/ext/testdata/import_invalid_version.json new file mode 100644 index 0000000000..23bb8999d7 --- /dev/null +++ b/internal/ext/testdata/import_invalid_version.json @@ -0,0 +1 @@ +{"version":"5.0","namespace":"default"} diff --git a/internal/ext/testdata/import_no_attachment.json b/internal/ext/testdata/import_no_attachment.json new file mode 100644 index 0000000000..c3d9fb3bed --- /dev/null +++ b/internal/ext/testdata/import_no_attachment.json @@ -0,0 +1 @@ +{"flags":[{"key":"flag1","name":"flag1","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","description":"variant description"}],"rules":[{"segment":"segment1","rank":1,"distributions":[{"variant":"variant1","rollout":100}]}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz"}]}]} diff --git a/internal/ext/testdata/import_rule_multiple_segments.json b/internal/ext/testdata/import_rule_multiple_segments.json new file mode 100644 index 0000000000..93ae2e1db7 --- /dev/null +++ b/internal/ext/testdata/import_rule_multiple_segments.json @@ -0,0 +1 @@ +{"flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","description":"variant description","attachment":{"pi":3.141,"happy":true,"name":"Niels","answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}}],"rules":[{"segment":{"keys":["segment1"],"operator":"OR_SEGMENT_OPERATOR"},"distributions":[{"variant":"variant1","rollout":100}]}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz"}]}]} diff --git a/internal/ext/testdata/import_single_namespace_foo.json b/internal/ext/testdata/import_single_namespace_foo.json new file mode 100644 index 0000000000..3daedb0a99 --- /dev/null +++ b/internal/ext/testdata/import_single_namespace_foo.json @@ -0,0 +1 @@ +{"namespace":"foo","flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","description":"variant description","attachment":{"pi":3.141,"happy":true,"name":"Niels","answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}}],"rules":[{"segment":"segment1","rank":1,"distributions":[{"variant":"variant1","rollout":100}]}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz"}]}]} diff --git a/internal/ext/testdata/import_two_namespaces_default_and_foo.json b/internal/ext/testdata/import_two_namespaces_default_and_foo.json new file mode 100644 index 0000000000..a8101d4781 --- /dev/null +++ b/internal/ext/testdata/import_two_namespaces_default_and_foo.json @@ -0,0 +1,2 @@ +{"namespace":"foo","flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","description":"variant description","attachment":{"pi":3.141,"happy":true,"name":"Niels","answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}}],"rules":[{"segment":"segment1","rank":1,"distributions":[{"variant":"variant1","rollout":100}]}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz"}]}]} +{"flags":[{"key":"flag3","name":"flag3","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true},{"key":"flag4","name":"flag4","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"segment2","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment2","name":"segment2","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"length","operator":"eq","value":"short"}]}]} diff --git a/internal/ext/testdata/import_v1.json b/internal/ext/testdata/import_v1.json new file mode 100644 index 0000000000..bad0a84dcb --- /dev/null +++ b/internal/ext/testdata/import_v1.json @@ -0,0 +1 @@ +{"version":"1.0","flags":[{"key":"flag1","name":"flag1","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","description":"variant description","attachment":{"pi":3.141,"happy":true,"name":"Niels","answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}}],"rules":[{"segment":"segment1","rank":1,"distributions":[{"variant":"variant1","rollout":100}]}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz"}]}]} diff --git a/internal/ext/testdata/import_v1_1.json b/internal/ext/testdata/import_v1_1.json new file mode 100644 index 0000000000..48382a0974 --- /dev/null +++ b/internal/ext/testdata/import_v1_1.json @@ -0,0 +1 @@ +{"version":"1.1","flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","description":"variant description","attachment":{"pi":3.141,"happy":true,"name":"Niels","answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}}],"rules":[{"segment":"segment1","rank":1,"distributions":[{"variant":"variant1","rollout":100}]}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz"}]}]} diff --git a/internal/ext/testdata/import_v1_flag_type_not_supported.json b/internal/ext/testdata/import_v1_flag_type_not_supported.json new file mode 100644 index 0000000000..07734fb73b --- /dev/null +++ b/internal/ext/testdata/import_v1_flag_type_not_supported.json @@ -0,0 +1 @@ +{"version":"1.0","flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true}]} diff --git a/internal/ext/testdata/import_v1_rollouts_not_supported.json b/internal/ext/testdata/import_v1_rollouts_not_supported.json new file mode 100644 index 0000000000..e766b87823 --- /dev/null +++ b/internal/ext/testdata/import_v1_rollouts_not_supported.json @@ -0,0 +1 @@ +{"version":"1.0","flags":[{"key":"flag1","name":"flag1","description":"description","enabled":false,"rollouts":[{"description":"some rollout","threshold":{"percentage":50,"value":true}}]}]} diff --git a/internal/ext/testdata/import_yaml_stream_all_unique_namespaces.json b/internal/ext/testdata/import_yaml_stream_all_unique_namespaces.json new file mode 100644 index 0000000000..833d45429c --- /dev/null +++ b/internal/ext/testdata/import_yaml_stream_all_unique_namespaces.json @@ -0,0 +1,3 @@ +{"namespace":"foo","flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","description":"variant description","attachment":{"pi":3.141,"happy":true,"name":"Niels","answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}}],"rules":[{"segment":"segment1","rank":1,"distributions":[{"variant":"variant1","rollout":100}]}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz"}]}]} +{"namespace":"bar","flags":[{"key":"flag3","name":"flag3","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true},{"key":"flag4","name":"flag4","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"segment2","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment2","name":"segment2","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"length","operator":"eq","value":"short"}]}]} +{"flags":[{"key":"flag3","name":"flag3","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true},{"key":"flag4","name":"flag4","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"segment2","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment2","name":"segment2","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"length","operator":"eq","value":"short"}]}]} diff --git a/internal/ext/testdata/import_yaml_stream_default_namespace.json b/internal/ext/testdata/import_yaml_stream_default_namespace.json new file mode 100644 index 0000000000..830e12925d --- /dev/null +++ b/internal/ext/testdata/import_yaml_stream_default_namespace.json @@ -0,0 +1,2 @@ +{"flags":[{"key":"flag1","name":"flag1","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true,"variants":[{"key":"variant1","name":"variant1","description":"variant description","attachment":{"pi":3.141,"happy":true,"name":"Niels","answer":{"everything":42},"list":[1,0,2],"object":{"currency":"USD","value":42.99}}}],"rules":[{"segment":"segment1","rank":1,"distributions":[{"variant":"variant1","rollout":100}]}]},{"key":"flag2","name":"flag2","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"internal_users","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment1","name":"segment1","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"fizz","operator":"neq","value":"buzz"}]}]} +{"flags":[{"key":"flag3","name":"flag3","type":"VARIANT_FLAG_TYPE","description":"description","enabled":true},{"key":"flag4","name":"flag4","type":"BOOLEAN_FLAG_TYPE","description":"a boolean flag","enabled":false,"rollouts":[{"description":"enabled for internal users","segment":{"key":"segment2","value":true}},{"description":"enabled for 50%","threshold":{"percentage":50,"value":true}}]}],"segments":[{"key":"segment2","name":"segment2","match_type":"ANY_MATCH_TYPE","description":"description","constraints":[{"type":"STRING_COMPARISON_TYPE","property":"length","operator":"eq","value":"short"}]}]} From 388b885a63dbc7b7b6f3126856efcc48902239aa Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Tue, 7 Nov 2023 18:23:08 +0000 Subject: [PATCH 10/20] fix(storage/fs/oci): update source for new oci.Store interface --- internal/storage/fs/oci/source.go | 9 ++++++--- internal/storage/fs/oci/source_test.go | 6 +++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/internal/storage/fs/oci/source.go b/internal/storage/fs/oci/source.go index 3e5906ca12..0a8d6ee99d 100644 --- a/internal/storage/fs/oci/source.go +++ b/internal/storage/fs/oci/source.go @@ -17,19 +17,22 @@ type Source struct { logger *zap.Logger interval time.Duration + store *oci.Store + ref oci.Reference + curSnap *storagefs.StoreSnapshot curDigest digest.Digest - store *oci.Store } // NewSource constructs and configures a Source. // The source uses the connection and credential details provided to build // *storagefs.StoreSnapshot implementations around a target git repository. -func NewSource(logger *zap.Logger, store *oci.Store, opts ...containers.Option[Source]) (_ *Source, err error) { +func NewSource(logger *zap.Logger, store *oci.Store, ref oci.Reference, opts ...containers.Option[Source]) (_ *Source, err error) { src := &Source{ logger: logger, interval: 30 * time.Second, store: store, + ref: ref, } containers.ApplyAll(src, opts...) @@ -50,7 +53,7 @@ func (s *Source) String() string { // Get builds a single instance of an *storagefs.StoreSnapshot func (s *Source) Get(context.Context) (*storagefs.StoreSnapshot, error) { - resp, err := s.store.Fetch(context.Background(), oci.IfNoMatch(s.curDigest)) + resp, err := s.store.Fetch(context.Background(), s.ref, oci.IfNoMatch(s.curDigest)) if err != nil { return nil, err } diff --git a/internal/storage/fs/oci/source_test.go b/internal/storage/fs/oci/source_test.go index 9d72c5c975..7ff1abcd14 100644 --- a/internal/storage/fs/oci/source_test.go +++ b/internal/storage/fs/oci/source_test.go @@ -91,11 +91,15 @@ func testSource(t *testing.T) (*Source, oras.Target) { layer("production", `{"namespace":"production"}`, fliptoci.MediaTypeFliptNamespace), ) - store, err := fliptoci.NewStore(zaptest.NewLogger(t), fmt.Sprintf("flipt://local/%s:latest", repo), fliptoci.WithBundleDir(dir)) + store, err := fliptoci.NewStore(zaptest.NewLogger(t), fliptoci.WithBundleDir(dir)) + require.NoError(t, err) + + ref, err := fliptoci.ParseReference(fmt.Sprintf("flipt://local/%s:latest", repo)) require.NoError(t, err) source, err := NewSource(zaptest.NewLogger(t), store, + ref, WithPollInterval(time.Second)) require.NoError(t, err) From 1c8fffdd6a25e1ccefb6f0b31f8d7c96a2788b9e Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Tue, 7 Nov 2023 18:25:56 +0000 Subject: [PATCH 11/20] fix(sql/test): use new import encoding signature --- internal/storage/sql/evaluation_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/storage/sql/evaluation_test.go b/internal/storage/sql/evaluation_test.go index 3074aa717a..38951bed91 100644 --- a/internal/storage/sql/evaluation_test.go +++ b/internal/storage/sql/evaluation_test.go @@ -881,7 +881,7 @@ func Benchmark_EvaluationV1AndV2(b *testing.B) { require.NoError(t, err) - err = importer.Import(context.TODO(), reader) + err = importer.Import(context.TODO(), ext.EncodingYML, reader) require.NoError(t, err) flagKeys := make([]string, 0, 10) From 59f0d54e602c57cfd2e3df01347a5fa3a1cce3a9 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Tue, 7 Nov 2023 18:26:17 +0000 Subject: [PATCH 12/20] chore(oci): make constants for possible schemes --- internal/oci/file.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/oci/file.go b/internal/oci/file.go index 00a282f0dc..a198aed2d6 100644 --- a/internal/oci/file.go +++ b/internal/oci/file.go @@ -30,6 +30,12 @@ import ( "oras.land/oras-go/v2/registry/remote" ) +const ( + SchemeHTTP = "http" + SchemeHTTPS = "https" + SchemeFlipt = "flipt" +) + // Store is a type which can retrieve Flipt feature files from a target repository and reference // Repositories can be local (OCI layout directories on the filesystem) or a remote registry type Store struct { @@ -101,12 +107,12 @@ func ParseReference(repository string) (Reference, error) { // support empty scheme as remote and https if !match { repository = scheme - scheme = "https" + scheme = SchemeHTTPS } if !strings.Contains(repository, "/") { repository = "local/" + repository - scheme = "flipt" + scheme = SchemeFlipt } ref, err := registry.ParseReference(repository) @@ -115,8 +121,8 @@ func ParseReference(repository string) (Reference, error) { } switch scheme { - case "http", "https": - case "flipt": + case SchemeHTTP, SchemeHTTPS: + case SchemeFlipt: if ref.Registry != "local" { return Reference{}, fmt.Errorf("unexpected local reference: %q", ref) } @@ -132,7 +138,7 @@ func ParseReference(repository string) (Reference, error) { func (s *Store) getTarget(ctx context.Context, ref Reference) (oras.Target, error) { switch ref.Scheme { - case "http", "https": + case SchemeHTTP, SchemeHTTPS: remote, err := remote.NewRepository(fmt.Sprintf("%s/%s", ref.Registry, ref.Repository)) if err != nil { return nil, err @@ -141,7 +147,7 @@ func (s *Store) getTarget(ctx context.Context, ref Reference) (oras.Target, erro remote.PlainHTTP = ref.Scheme == "http" return remote, nil - case "flipt": + case SchemeFlipt: // build the store once to ensure it is valid bundleDir := path.Join(s.opts.bundleDir, ref.Repository) _, err := oci.New(bundleDir) From 9d3165a7c488a4b848c268a7bea531235fb8f66d Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Tue, 7 Nov 2023 18:28:50 +0000 Subject: [PATCH 13/20] chore(ext): remove unnecessary conversions --- internal/ext/exporter_test.go | 3 ++- internal/ext/importer_test.go | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/ext/exporter_test.go b/internal/ext/exporter_test.go index 41028d13e8..d01582549d 100644 --- a/internal/ext/exporter_test.go +++ b/internal/ext/exporter_test.go @@ -3,6 +3,7 @@ package ext import ( "bytes" "context" + "errors" "fmt" "io" "os" @@ -777,7 +778,7 @@ func TestExport(t *testing.T) { ferr := found.Decode(&fnd) require.Equal(t, eerr, ferr) - if ferr == io.EOF { + if errors.Is(ferr, io.EOF) { break } require.NoError(t, ferr) diff --git a/internal/ext/importer_test.go b/internal/ext/importer_test.go index 054412a3ec..7d2ab6eb2e 100644 --- a/internal/ext/importer_test.go +++ b/internal/ext/importer_test.go @@ -693,7 +693,7 @@ func TestImport(t *testing.T) { assert.NoError(t, err) defer in.Close() - err = importer.Import(context.Background(), Encoding(ext), in) + err = importer.Import(context.Background(), ext, in) assert.NoError(t, err) assert.Equal(t, tc.expected, creator) @@ -728,7 +728,7 @@ func TestImport_InvalidVersion(t *testing.T) { assert.NoError(t, err) defer in.Close() - err = importer.Import(context.Background(), Encoding(ext), in) + err = importer.Import(context.Background(), ext, in) assert.EqualError(t, err, "unsupported version: 5.0") } } @@ -744,7 +744,7 @@ func TestImport_FlagType_LTVersion1_1(t *testing.T) { assert.NoError(t, err) defer in.Close() - err = importer.Import(context.Background(), Encoding(ext), in) + err = importer.Import(context.Background(), ext, in) assert.EqualError(t, err, "flag.type is supported in version >=1.1, found 1.0") } } @@ -760,7 +760,7 @@ func TestImport_Rollouts_LTVersion1_1(t *testing.T) { assert.NoError(t, err) defer in.Close() - err = importer.Import(context.Background(), Encoding(ext), in) + err = importer.Import(context.Background(), ext, in) assert.EqualError(t, err, "flag.rollouts is supported in version >=1.1, found 1.0") } } @@ -823,7 +823,7 @@ func TestImport_Namespaces_Mix_And_Match(t *testing.T) { assert.NoError(t, err) defer in.Close() - err = importer.Import(context.Background(), Encoding(ext), in) + err = importer.Import(context.Background(), ext, in) assert.NoError(t, err) assert.Len(t, creator.getNSReqs, tc.expectedGetNSReqs) From 60b57eea402626f1a0fb296c36cac7651a882506 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Tue, 7 Nov 2023 18:29:13 +0000 Subject: [PATCH 14/20] chore(oci): remove uncessary context argument --- internal/oci/file.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/oci/file.go b/internal/oci/file.go index a198aed2d6..c377b18c1b 100644 --- a/internal/oci/file.go +++ b/internal/oci/file.go @@ -117,7 +117,7 @@ func ParseReference(repository string) (Reference, error) { ref, err := registry.ParseReference(repository) if err != nil { - return Reference{}, nil + return Reference{}, err } switch scheme { @@ -136,7 +136,7 @@ func ParseReference(repository string) (Reference, error) { }, nil } -func (s *Store) getTarget(ctx context.Context, ref Reference) (oras.Target, error) { +func (s *Store) getTarget(ref Reference) (oras.Target, error) { switch ref.Scheme { case SchemeHTTP, SchemeHTTPS: remote, err := remote.NewRepository(fmt.Sprintf("%s/%s", ref.Registry, ref.Repository)) @@ -198,7 +198,7 @@ func (s *Store) Fetch(ctx context.Context, ref Reference, opts ...containers.Opt var options FetchOptions containers.ApplyAll(&options, opts...) - store, err := s.getTarget(ctx, ref) + store, err := s.getTarget(ref) if err != nil { return nil, err } @@ -365,7 +365,7 @@ func (s *Store) List(ctx context.Context) (bundles []Bundle, _ error) { // Build bundles the target directory Flipt feature state into the target configured on the Store // It returns a Bundle which contains metadata regarding the resulting bundle details func (s *Store) Build(ctx context.Context, src fs.FS, ref Reference) (Bundle, error) { - store, err := s.getTarget(ctx, ref) + store, err := s.getTarget(ref) if err != nil { return Bundle{}, err } From 7c8ecdc62c174f39c5be5d760bdd04778010d47c Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Wed, 8 Nov 2023 11:48:12 +0000 Subject: [PATCH 15/20] test(storage/fs): unknown extension returns an error --- internal/storage/fs/snapshot.go | 12 ++++++---- internal/storage/fs/snapshot_test.go | 24 ++++++++++++------- internal/storage/fs/store_test.go | 4 ++-- .../boolean_flag_segment}/features.yml | 0 .../fs/testdata/invalid/extension/.flipt.yml | 3 +++ .../invalid/extension/features.unknown | 1 + .../variant_flag_distribution}/features.yml | 0 .../variant_flag_segment}/features.yml | 0 .../valid}/empty_features/features.yml | 0 .../valid/explicit_index}/.flipt.yml | 0 .../valid/explicit_index}/prod/features.yml | 0 .../explicit_index}/prod/prod.features.yml | 0 .../valid/explicit_index}/prod/prod.yml | 0 .../explicit_index}/sandbox/features.yaml | 0 .../sandbox/sandbox.features.yaml | 0 .../valid/explicit_index}/sandbox/sandbox.yml | 0 .../explicit_index}/staging/features.yml | 0 .../staging/staging.features.yml | 0 .../valid/explicit_index}/staging/staging.yml | 0 .../valid/implicit_index}/prod/features.yml | 0 .../implicit_index}/prod/prod.features.yaml | 0 .../valid/implicit_index}/prod/prod.yml | 0 .../implicit_index}/staging/features.yml | 0 .../staging/sandbox/features.yaml | 0 .../staging/sandbox/sandbox.features.yml | 0 .../staging/sandbox/sandbox.yml | 0 .../staging/staging.features.yaml | 0 .../valid/implicit_index}/staging/staging.yml | 0 .../valid}/yaml_stream/features.yaml | 0 29 files changed, 30 insertions(+), 14 deletions(-) rename internal/storage/fs/{fixtures/invalid_boolean_flag_segment => testdata/invalid/boolean_flag_segment}/features.yml (100%) create mode 100644 internal/storage/fs/testdata/invalid/extension/.flipt.yml create mode 100644 internal/storage/fs/testdata/invalid/extension/features.unknown rename internal/storage/fs/{fixtures/invalid_variant_flag_distribution => testdata/invalid/variant_flag_distribution}/features.yml (100%) rename internal/storage/fs/{fixtures/invalid_variant_flag_segment => testdata/invalid/variant_flag_segment}/features.yml (100%) rename internal/storage/fs/{fixtures => testdata/valid}/empty_features/features.yml (100%) rename internal/storage/fs/{fixtures/fswithindex => testdata/valid/explicit_index}/.flipt.yml (100%) rename internal/storage/fs/{fixtures/fswithindex => testdata/valid/explicit_index}/prod/features.yml (100%) rename internal/storage/fs/{fixtures/fswithindex => testdata/valid/explicit_index}/prod/prod.features.yml (100%) rename internal/storage/fs/{fixtures/fswithindex => testdata/valid/explicit_index}/prod/prod.yml (100%) rename internal/storage/fs/{fixtures/fswithindex => testdata/valid/explicit_index}/sandbox/features.yaml (100%) rename internal/storage/fs/{fixtures/fswithindex => testdata/valid/explicit_index}/sandbox/sandbox.features.yaml (100%) rename internal/storage/fs/{fixtures/fswithindex => testdata/valid/explicit_index}/sandbox/sandbox.yml (100%) rename internal/storage/fs/{fixtures/fswithindex => testdata/valid/explicit_index}/staging/features.yml (100%) rename internal/storage/fs/{fixtures/fswithindex => testdata/valid/explicit_index}/staging/staging.features.yml (100%) rename internal/storage/fs/{fixtures/fswithindex => testdata/valid/explicit_index}/staging/staging.yml (100%) rename internal/storage/fs/{fixtures/fswithoutindex => testdata/valid/implicit_index}/prod/features.yml (100%) rename internal/storage/fs/{fixtures/fswithoutindex => testdata/valid/implicit_index}/prod/prod.features.yaml (100%) rename internal/storage/fs/{fixtures/fswithoutindex => testdata/valid/implicit_index}/prod/prod.yml (100%) rename internal/storage/fs/{fixtures/fswithoutindex => testdata/valid/implicit_index}/staging/features.yml (100%) rename internal/storage/fs/{fixtures/fswithoutindex => testdata/valid/implicit_index}/staging/sandbox/features.yaml (100%) rename internal/storage/fs/{fixtures/fswithoutindex => testdata/valid/implicit_index}/staging/sandbox/sandbox.features.yml (100%) rename internal/storage/fs/{fixtures/fswithoutindex => testdata/valid/implicit_index}/staging/sandbox/sandbox.yml (100%) rename internal/storage/fs/{fixtures/fswithoutindex => testdata/valid/implicit_index}/staging/staging.features.yaml (100%) rename internal/storage/fs/{fixtures/fswithoutindex => testdata/valid/implicit_index}/staging/staging.yml (100%) rename internal/storage/fs/{fixtures => testdata/valid}/yaml_stream/features.yaml (100%) diff --git a/internal/storage/fs/snapshot.go b/internal/storage/fs/snapshot.go index f3b98fa72c..4541ab902f 100644 --- a/internal/storage/fs/snapshot.go +++ b/internal/storage/fs/snapshot.go @@ -184,10 +184,6 @@ func documentsFromFile(fi fs.File) ([]*ext.Document, error) { buf := &bytes.Buffer{} reader := io.TeeReader(fi, buf) - if err := validator.Validate(stat.Name(), reader); err != nil { - return nil, err - } - var docs []*ext.Document extn := filepath.Ext(stat.Name()) @@ -202,6 +198,14 @@ func documentsFromFile(fi fs.File) ([]*ext.Document, error) { return nil, fmt.Errorf("unexpected extension: %q", extn) } + // validate after we have checked supported + // extensions but before we attempt to decode the + // buffers contents to ensure we fill the buffer + // via the TeeReader + if err := validator.Validate(stat.Name(), reader); err != nil { + return nil, err + } + for { doc := &ext.Document{} if err := decode(doc); err != nil { diff --git a/internal/storage/fs/snapshot_test.go b/internal/storage/fs/snapshot_test.go index 29da18f24f..2af6928739 100644 --- a/internal/storage/fs/snapshot_test.go +++ b/internal/storage/fs/snapshot_test.go @@ -15,11 +15,19 @@ import ( "go.uber.org/zap/zaptest" ) -//go:embed all:fixtures +//go:embed all:testdata var testdata embed.FS +func TestSnapshotFromFS_Invalid_Extension(t *testing.T) { + dir, err := fs.Sub(testdata, "testdata/invalid/extension") + require.NoError(t, err) + + _, err = SnapshotFromFS(zaptest.NewLogger(t), dir) + require.Equal(t, errors.New("unexpected extension: \".unknown\""), err) +} + func TestFSWithIndex(t *testing.T) { - fwi, _ := fs.Sub(testdata, "fixtures/fswithindex") + fwi, _ := fs.Sub(testdata, "testdata/valid/explicit_index") ss, err := SnapshotFromFS(zaptest.NewLogger(t), fwi) require.NoError(t, err) @@ -681,7 +689,7 @@ type FSWithoutIndexSuite struct { } func TestFSWithoutIndex(t *testing.T) { - fwoi, _ := fs.Sub(testdata, "fixtures/fswithoutindex") + fwoi, _ := fs.Sub(testdata, "testdata/valid/implicit_index") ss, err := SnapshotFromFS(zaptest.NewLogger(t), fwoi) require.NoError(t, err) @@ -1610,25 +1618,25 @@ func (fis *FSWithoutIndexSuite) TestListAndGetRules() { } func TestFS_Invalid_VariantFlag_Segment(t *testing.T) { - fs, _ := fs.Sub(testdata, "fixtures/invalid_variant_flag_segment") + fs, _ := fs.Sub(testdata, "testdata/invalid/variant_flag_segment") _, err := SnapshotFromFS(zaptest.NewLogger(t), fs) require.EqualError(t, err, "flag fruit/apple rule 1 references unknown segment \"unknown\"") } func TestFS_Invalid_VariantFlag_Distribution(t *testing.T) { - fs, _ := fs.Sub(testdata, "fixtures/invalid_variant_flag_distribution") + fs, _ := fs.Sub(testdata, "testdata/invalid/variant_flag_distribution") _, err := SnapshotFromFS(zaptest.NewLogger(t), fs) require.EqualError(t, err, "flag fruit/apple rule 1 references unknown variant \"braeburn\"") } func TestFS_Invalid_BooleanFlag_Segment(t *testing.T) { - fs, _ := fs.Sub(testdata, "fixtures/invalid_boolean_flag_segment") + fs, _ := fs.Sub(testdata, "testdata/invalid/boolean_flag_segment") _, err := SnapshotFromFS(zaptest.NewLogger(t), fs) require.EqualError(t, err, "flag fruit/apple rule 1 references unknown segment \"unknown\"") } func TestFS_Empty_Features_File(t *testing.T) { - fs, _ := fs.Sub(testdata, "fixtures/empty_features") + fs, _ := fs.Sub(testdata, "testdata/valid/empty_features") ss, err := SnapshotFromFS(zaptest.NewLogger(t), fs) require.NoError(t, err) @@ -1646,7 +1654,7 @@ func TestFS_Empty_Features_File(t *testing.T) { } func TestFS_YAML_Stream(t *testing.T) { - fs, _ := fs.Sub(testdata, "fixtures/yaml_stream") + fs, _ := fs.Sub(testdata, "testdata/valid/yaml_stream") ss, err := SnapshotFromFS(zaptest.NewLogger(t), fs) require.NoError(t, err) diff --git a/internal/storage/fs/store_test.go b/internal/storage/fs/store_test.go index 296f503fa3..41cae2631b 100644 --- a/internal/storage/fs/store_test.go +++ b/internal/storage/fs/store_test.go @@ -16,7 +16,7 @@ func Test_Store(t *testing.T) { logger = zaptest.NewLogger(t) notify = make(chan struct{}) source = source{ - get: mustSub(t, testdata, "fixtures/fswithindex"), + get: mustSub(t, testdata, "testdata/valid/explicit_index"), ch: make(chan *StoreSnapshot), } ) @@ -36,7 +36,7 @@ func Test_Store(t *testing.T) { suite.Run(t, &FSIndexSuite{store: store}) // update snapshot by sending fs without index - source.ch <- mustSub(t, testdata, "fixtures/fswithoutindex") + source.ch <- mustSub(t, testdata, "testdata/valid/implicit_index") // wait for update to apply <-notify diff --git a/internal/storage/fs/fixtures/invalid_boolean_flag_segment/features.yml b/internal/storage/fs/testdata/invalid/boolean_flag_segment/features.yml similarity index 100% rename from internal/storage/fs/fixtures/invalid_boolean_flag_segment/features.yml rename to internal/storage/fs/testdata/invalid/boolean_flag_segment/features.yml diff --git a/internal/storage/fs/testdata/invalid/extension/.flipt.yml b/internal/storage/fs/testdata/invalid/extension/.flipt.yml new file mode 100644 index 0000000000..715f77329c --- /dev/null +++ b/internal/storage/fs/testdata/invalid/extension/.flipt.yml @@ -0,0 +1,3 @@ +version: "1.0" +include: +- features.unknown diff --git a/internal/storage/fs/testdata/invalid/extension/features.unknown b/internal/storage/fs/testdata/invalid/extension/features.unknown new file mode 100644 index 0000000000..e40d061d8f --- /dev/null +++ b/internal/storage/fs/testdata/invalid/extension/features.unknown @@ -0,0 +1 @@ +whatisthisformat? diff --git a/internal/storage/fs/fixtures/invalid_variant_flag_distribution/features.yml b/internal/storage/fs/testdata/invalid/variant_flag_distribution/features.yml similarity index 100% rename from internal/storage/fs/fixtures/invalid_variant_flag_distribution/features.yml rename to internal/storage/fs/testdata/invalid/variant_flag_distribution/features.yml diff --git a/internal/storage/fs/fixtures/invalid_variant_flag_segment/features.yml b/internal/storage/fs/testdata/invalid/variant_flag_segment/features.yml similarity index 100% rename from internal/storage/fs/fixtures/invalid_variant_flag_segment/features.yml rename to internal/storage/fs/testdata/invalid/variant_flag_segment/features.yml diff --git a/internal/storage/fs/fixtures/empty_features/features.yml b/internal/storage/fs/testdata/valid/empty_features/features.yml similarity index 100% rename from internal/storage/fs/fixtures/empty_features/features.yml rename to internal/storage/fs/testdata/valid/empty_features/features.yml diff --git a/internal/storage/fs/fixtures/fswithindex/.flipt.yml b/internal/storage/fs/testdata/valid/explicit_index/.flipt.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithindex/.flipt.yml rename to internal/storage/fs/testdata/valid/explicit_index/.flipt.yml diff --git a/internal/storage/fs/fixtures/fswithindex/prod/features.yml b/internal/storage/fs/testdata/valid/explicit_index/prod/features.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithindex/prod/features.yml rename to internal/storage/fs/testdata/valid/explicit_index/prod/features.yml diff --git a/internal/storage/fs/fixtures/fswithindex/prod/prod.features.yml b/internal/storage/fs/testdata/valid/explicit_index/prod/prod.features.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithindex/prod/prod.features.yml rename to internal/storage/fs/testdata/valid/explicit_index/prod/prod.features.yml diff --git a/internal/storage/fs/fixtures/fswithindex/prod/prod.yml b/internal/storage/fs/testdata/valid/explicit_index/prod/prod.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithindex/prod/prod.yml rename to internal/storage/fs/testdata/valid/explicit_index/prod/prod.yml diff --git a/internal/storage/fs/fixtures/fswithindex/sandbox/features.yaml b/internal/storage/fs/testdata/valid/explicit_index/sandbox/features.yaml similarity index 100% rename from internal/storage/fs/fixtures/fswithindex/sandbox/features.yaml rename to internal/storage/fs/testdata/valid/explicit_index/sandbox/features.yaml diff --git a/internal/storage/fs/fixtures/fswithindex/sandbox/sandbox.features.yaml b/internal/storage/fs/testdata/valid/explicit_index/sandbox/sandbox.features.yaml similarity index 100% rename from internal/storage/fs/fixtures/fswithindex/sandbox/sandbox.features.yaml rename to internal/storage/fs/testdata/valid/explicit_index/sandbox/sandbox.features.yaml diff --git a/internal/storage/fs/fixtures/fswithindex/sandbox/sandbox.yml b/internal/storage/fs/testdata/valid/explicit_index/sandbox/sandbox.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithindex/sandbox/sandbox.yml rename to internal/storage/fs/testdata/valid/explicit_index/sandbox/sandbox.yml diff --git a/internal/storage/fs/fixtures/fswithindex/staging/features.yml b/internal/storage/fs/testdata/valid/explicit_index/staging/features.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithindex/staging/features.yml rename to internal/storage/fs/testdata/valid/explicit_index/staging/features.yml diff --git a/internal/storage/fs/fixtures/fswithindex/staging/staging.features.yml b/internal/storage/fs/testdata/valid/explicit_index/staging/staging.features.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithindex/staging/staging.features.yml rename to internal/storage/fs/testdata/valid/explicit_index/staging/staging.features.yml diff --git a/internal/storage/fs/fixtures/fswithindex/staging/staging.yml b/internal/storage/fs/testdata/valid/explicit_index/staging/staging.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithindex/staging/staging.yml rename to internal/storage/fs/testdata/valid/explicit_index/staging/staging.yml diff --git a/internal/storage/fs/fixtures/fswithoutindex/prod/features.yml b/internal/storage/fs/testdata/valid/implicit_index/prod/features.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithoutindex/prod/features.yml rename to internal/storage/fs/testdata/valid/implicit_index/prod/features.yml diff --git a/internal/storage/fs/fixtures/fswithoutindex/prod/prod.features.yaml b/internal/storage/fs/testdata/valid/implicit_index/prod/prod.features.yaml similarity index 100% rename from internal/storage/fs/fixtures/fswithoutindex/prod/prod.features.yaml rename to internal/storage/fs/testdata/valid/implicit_index/prod/prod.features.yaml diff --git a/internal/storage/fs/fixtures/fswithoutindex/prod/prod.yml b/internal/storage/fs/testdata/valid/implicit_index/prod/prod.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithoutindex/prod/prod.yml rename to internal/storage/fs/testdata/valid/implicit_index/prod/prod.yml diff --git a/internal/storage/fs/fixtures/fswithoutindex/staging/features.yml b/internal/storage/fs/testdata/valid/implicit_index/staging/features.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithoutindex/staging/features.yml rename to internal/storage/fs/testdata/valid/implicit_index/staging/features.yml diff --git a/internal/storage/fs/fixtures/fswithoutindex/staging/sandbox/features.yaml b/internal/storage/fs/testdata/valid/implicit_index/staging/sandbox/features.yaml similarity index 100% rename from internal/storage/fs/fixtures/fswithoutindex/staging/sandbox/features.yaml rename to internal/storage/fs/testdata/valid/implicit_index/staging/sandbox/features.yaml diff --git a/internal/storage/fs/fixtures/fswithoutindex/staging/sandbox/sandbox.features.yml b/internal/storage/fs/testdata/valid/implicit_index/staging/sandbox/sandbox.features.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithoutindex/staging/sandbox/sandbox.features.yml rename to internal/storage/fs/testdata/valid/implicit_index/staging/sandbox/sandbox.features.yml diff --git a/internal/storage/fs/fixtures/fswithoutindex/staging/sandbox/sandbox.yml b/internal/storage/fs/testdata/valid/implicit_index/staging/sandbox/sandbox.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithoutindex/staging/sandbox/sandbox.yml rename to internal/storage/fs/testdata/valid/implicit_index/staging/sandbox/sandbox.yml diff --git a/internal/storage/fs/fixtures/fswithoutindex/staging/staging.features.yaml b/internal/storage/fs/testdata/valid/implicit_index/staging/staging.features.yaml similarity index 100% rename from internal/storage/fs/fixtures/fswithoutindex/staging/staging.features.yaml rename to internal/storage/fs/testdata/valid/implicit_index/staging/staging.features.yaml diff --git a/internal/storage/fs/fixtures/fswithoutindex/staging/staging.yml b/internal/storage/fs/testdata/valid/implicit_index/staging/staging.yml similarity index 100% rename from internal/storage/fs/fixtures/fswithoutindex/staging/staging.yml rename to internal/storage/fs/testdata/valid/implicit_index/staging/staging.yml diff --git a/internal/storage/fs/fixtures/yaml_stream/features.yaml b/internal/storage/fs/testdata/valid/yaml_stream/features.yaml similarity index 100% rename from internal/storage/fs/fixtures/yaml_stream/features.yaml rename to internal/storage/fs/testdata/valid/yaml_stream/features.yaml From 86bb9006e80ff4a7e6590b4e0dfed062a851846c Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Wed, 8 Nov 2023 11:56:57 +0000 Subject: [PATCH 16/20] test(storage/fs): ensure count rules returns as expected --- internal/storage/fs/snapshot_test.go | 33 ++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/internal/storage/fs/snapshot_test.go b/internal/storage/fs/snapshot_test.go index 2af6928739..3570c4e013 100644 --- a/internal/storage/fs/snapshot_test.go +++ b/internal/storage/fs/snapshot_test.go @@ -683,6 +683,39 @@ func (fis *FSIndexSuite) TestListAndGetRules() { } } +func (fis *FSIndexSuite) TestCountRules() { + t := fis.T() + + testCases := []struct { + name string + namespace string + flagKey string + count uint64 + }{ + { + name: "Production", + namespace: "production", + flagKey: "prod-flag", + count: 1, + }, + { + name: "Sandbox", + namespace: "sandbox", + flagKey: "sandbox-flag", + count: 1, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + count, err := fis.store.CountRules(context.TODO(), tc.namespace, tc.flagKey) + require.NoError(t, err) + + assert.Equal(t, tc.count, count) + }) + } +} + type FSWithoutIndexSuite struct { suite.Suite store storage.Store From 720ad3625794dcc729f047d17f7a19beda4ef1ba Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Wed, 8 Nov 2023 12:12:04 +0000 Subject: [PATCH 17/20] test(storage/fs): ensure walk documents returns expected count --- internal/storage/fs/snapshot_test.go | 30 ++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/internal/storage/fs/snapshot_test.go b/internal/storage/fs/snapshot_test.go index 3570c4e013..ce05321cf2 100644 --- a/internal/storage/fs/snapshot_test.go +++ b/internal/storage/fs/snapshot_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "go.flipt.io/flipt/internal/ext" "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" "go.uber.org/zap/zaptest" @@ -26,6 +27,35 @@ func TestSnapshotFromFS_Invalid_Extension(t *testing.T) { require.Equal(t, errors.New("unexpected extension: \".unknown\""), err) } +func TestWalkDocuments(t *testing.T) { + for _, test := range []struct { + path string + count int + }{ + { + path: "testdata/valid/explicit_index", + count: 2, + }, + { + path: "testdata/valid/implicit_index", + count: 6, + }, + } { + t.Run(test.path, func(t *testing.T) { + src, err := fs.Sub(testdata, test.path) + require.NoError(t, err) + + var docs []*ext.Document + require.NoError(t, WalkDocuments(zaptest.NewLogger(t), src, func(d *ext.Document) error { + docs = append(docs, d) + return nil + })) + + assert.Len(t, docs, test.count) + }) + } +} + func TestFSWithIndex(t *testing.T) { fwi, _ := fs.Sub(testdata, "testdata/valid/explicit_index") From 5ca0e60663b1d72fe76a661575eeca664fc6b6a1 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Wed, 8 Nov 2023 12:46:42 +0000 Subject: [PATCH 18/20] test(storage/fs): add more cases around invalid formats --- internal/storage/fs/snapshot_test.go | 66 ++++++++++++------- .../fs/testdata/invalid/namespace/.flipt.yml | 3 + .../testdata/invalid/namespace/features.json | 1 + 3 files changed, 47 insertions(+), 23 deletions(-) create mode 100644 internal/storage/fs/testdata/invalid/namespace/.flipt.yml create mode 100644 internal/storage/fs/testdata/invalid/namespace/features.json diff --git a/internal/storage/fs/snapshot_test.go b/internal/storage/fs/snapshot_test.go index ce05321cf2..72d675197d 100644 --- a/internal/storage/fs/snapshot_test.go +++ b/internal/storage/fs/snapshot_test.go @@ -4,12 +4,15 @@ import ( "context" "embed" "errors" + "fmt" "io/fs" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + flipterrors "go.flipt.io/flipt/errors" + "go.flipt.io/flipt/internal/cue" "go.flipt.io/flipt/internal/ext" "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" @@ -19,12 +22,47 @@ import ( //go:embed all:testdata var testdata embed.FS -func TestSnapshotFromFS_Invalid_Extension(t *testing.T) { - dir, err := fs.Sub(testdata, "testdata/invalid/extension") - require.NoError(t, err) +func TestSnapshotFromFS_Invalid(t *testing.T) { + for _, test := range []struct { + path string + err error + }{ + { + path: "testdata/invalid/extension", + err: errors.New("unexpected extension: \".unknown\""), + }, + { + path: "testdata/invalid/variant_flag_segment", + err: flipterrors.ErrInvalid("flag fruit/apple rule 1 references unknown segment \"unknown\""), + }, + { + path: "testdata/invalid/variant_flag_distribution", + err: flipterrors.ErrInvalid("flag fruit/apple rule 1 references unknown variant \"braeburn\""), + }, + { + path: "testdata/invalid/boolean_flag_segment", + err: flipterrors.ErrInvalid("flag fruit/apple rule 1 references unknown segment \"unknown\""), + }, + { + path: "testdata/invalid/namespace", + err: errors.Join( + cue.Error{Message: "namespace: 2 errors in empty disjunction:", Location: cue.Location{File: "features.json", Line: 0}}, + cue.Error{Message: "namespace: conflicting values 1 and \"default\" (mismatched types int and string)", Location: cue.Location{File: "features.json", Line: 3}}, + cue.Error{Message: "namespace: conflicting values 1 and string (mismatched types int and string)", Location: cue.Location{File: "features.json", Line: 3}}, + ), + }, + } { + t.Run(test.path, func(t *testing.T) { + dir, err := fs.Sub(testdata, test.path) + require.NoError(t, err) + + _, err = SnapshotFromFS(zaptest.NewLogger(t), dir) + if !assert.Equal(t, test.err, err) { + fmt.Println(err) + } + }) + } - _, err = SnapshotFromFS(zaptest.NewLogger(t), dir) - require.Equal(t, errors.New("unexpected extension: \".unknown\""), err) } func TestWalkDocuments(t *testing.T) { @@ -1680,24 +1718,6 @@ func (fis *FSWithoutIndexSuite) TestListAndGetRules() { } } -func TestFS_Invalid_VariantFlag_Segment(t *testing.T) { - fs, _ := fs.Sub(testdata, "testdata/invalid/variant_flag_segment") - _, err := SnapshotFromFS(zaptest.NewLogger(t), fs) - require.EqualError(t, err, "flag fruit/apple rule 1 references unknown segment \"unknown\"") -} - -func TestFS_Invalid_VariantFlag_Distribution(t *testing.T) { - fs, _ := fs.Sub(testdata, "testdata/invalid/variant_flag_distribution") - _, err := SnapshotFromFS(zaptest.NewLogger(t), fs) - require.EqualError(t, err, "flag fruit/apple rule 1 references unknown variant \"braeburn\"") -} - -func TestFS_Invalid_BooleanFlag_Segment(t *testing.T) { - fs, _ := fs.Sub(testdata, "testdata/invalid/boolean_flag_segment") - _, err := SnapshotFromFS(zaptest.NewLogger(t), fs) - require.EqualError(t, err, "flag fruit/apple rule 1 references unknown segment \"unknown\"") -} - func TestFS_Empty_Features_File(t *testing.T) { fs, _ := fs.Sub(testdata, "testdata/valid/empty_features") ss, err := SnapshotFromFS(zaptest.NewLogger(t), fs) diff --git a/internal/storage/fs/testdata/invalid/namespace/.flipt.yml b/internal/storage/fs/testdata/invalid/namespace/.flipt.yml new file mode 100644 index 0000000000..b084b72a00 --- /dev/null +++ b/internal/storage/fs/testdata/invalid/namespace/.flipt.yml @@ -0,0 +1,3 @@ +version: "1.0" +include: +- "features.json" diff --git a/internal/storage/fs/testdata/invalid/namespace/features.json b/internal/storage/fs/testdata/invalid/namespace/features.json new file mode 100644 index 0000000000..0eb855b9fb --- /dev/null +++ b/internal/storage/fs/testdata/invalid/namespace/features.json @@ -0,0 +1 @@ +{"namespace":1} From 31800d888f5f6a901414ee48fe297a700e4c5c81 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Wed, 8 Nov 2023 17:46:04 +0000 Subject: [PATCH 19/20] chore(bundle): add build use string --- cmd/flipt/bundle.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/flipt/bundle.go b/cmd/flipt/bundle.go index c307f708c0..ceaa6b2d8e 100644 --- a/cmd/flipt/bundle.go +++ b/cmd/flipt/bundle.go @@ -21,7 +21,7 @@ func newBundleCommand() *cobra.Command { } cmd.AddCommand(&cobra.Command{ - Use: "build", + Use: "build [flags] ", Short: "Build a bundle", RunE: bundle.build, Args: cobra.ExactArgs(1), From fc59ef6d01c6695c7c29fb61c9f64b22c065769b Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Wed, 8 Nov 2023 17:48:48 +0000 Subject: [PATCH 20/20] fix(storage/fs): dont index beyond slice bounds --- .flipt.yml | 8 ++++++++ internal/storage/fs/snapshot.go | 2 +- internal/storage/fs/snapshot_test.go | 4 ++++ .../storage/fs/testdata/valid/exclude_index/.flipt.yml | 5 +++++ .../fs/testdata/valid/exclude_index/exclude.features.yml | 1 + .../fs/testdata/valid/exclude_index/include.features.yml | 1 + 6 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 .flipt.yml create mode 100644 internal/storage/fs/testdata/valid/exclude_index/.flipt.yml create mode 100644 internal/storage/fs/testdata/valid/exclude_index/exclude.features.yml create mode 100644 internal/storage/fs/testdata/valid/exclude_index/include.features.yml diff --git a/.flipt.yml b/.flipt.yml new file mode 100644 index 0000000000..d01d8b83f2 --- /dev/null +++ b/.flipt.yml @@ -0,0 +1,8 @@ +version: "1.0" +include: +- "**features.yml" +- "**features.yaml" +- "**.features.yml" +- "**.features.yaml" +exclude: +- internal/* diff --git a/internal/storage/fs/snapshot.go b/internal/storage/fs/snapshot.go index 4541ab902f..bfb8de5976 100644 --- a/internal/storage/fs/snapshot.go +++ b/internal/storage/fs/snapshot.go @@ -296,7 +296,7 @@ func listStateFiles(logger *zap.Logger, source fs.FS) ([]string, error) { } OUTER: - for i := range filenames { + for i := len(filenames) - 1; i >= 0; i-- { for _, glob := range excludes { if glob.Match(filenames[i]) { filenames = append(filenames[:i], filenames[i+1:]...) diff --git a/internal/storage/fs/snapshot_test.go b/internal/storage/fs/snapshot_test.go index 72d675197d..1a4334d71b 100644 --- a/internal/storage/fs/snapshot_test.go +++ b/internal/storage/fs/snapshot_test.go @@ -78,6 +78,10 @@ func TestWalkDocuments(t *testing.T) { path: "testdata/valid/implicit_index", count: 6, }, + { + path: "testdata/valid/exclude_index", + count: 1, + }, } { t.Run(test.path, func(t *testing.T) { src, err := fs.Sub(testdata, test.path) diff --git a/internal/storage/fs/testdata/valid/exclude_index/.flipt.yml b/internal/storage/fs/testdata/valid/exclude_index/.flipt.yml new file mode 100644 index 0000000000..3815dc6343 --- /dev/null +++ b/internal/storage/fs/testdata/valid/exclude_index/.flipt.yml @@ -0,0 +1,5 @@ +version: "1.0" +include: +- "*.features.yml" +exclude: +- "exclude.features.yml" diff --git a/internal/storage/fs/testdata/valid/exclude_index/exclude.features.yml b/internal/storage/fs/testdata/valid/exclude_index/exclude.features.yml new file mode 100644 index 0000000000..1a58bbd5fd --- /dev/null +++ b/internal/storage/fs/testdata/valid/exclude_index/exclude.features.yml @@ -0,0 +1 @@ +namespace: exclude diff --git a/internal/storage/fs/testdata/valid/exclude_index/include.features.yml b/internal/storage/fs/testdata/valid/exclude_index/include.features.yml new file mode 100644 index 0000000000..d107e555ba --- /dev/null +++ b/internal/storage/fs/testdata/valid/exclude_index/include.features.yml @@ -0,0 +1 @@ +namespace: include