Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: bucket prefix #3808

Merged
merged 10 commits into from
Jan 17, 2025
Merged

fix: bucket prefix #3808

merged 10 commits into from
Jan 17, 2025

Conversation

markphelps
Copy link
Collaborator

Should fix: #3807

Re: #2589

It seems gcp was adding the prefix twice, so it would make requests like:

https://storage.googleapis.com/flipt-test/productionproduction.flipt.yml

I assume this is because of us setting both the query param prefix when building the bucket URL and by setting prefix+key for all object requests.

I discovered that Go Cloud has support for prefixed buckets naturally: https://gocloud.dev/howto/blob/#prefix

Which now does the right thing and lists the contents at:

https://storage.googleapis.com/flipt-test/production/flipt.yml

I still need to test if this breaks S3 prefix though

@markphelps
chore: prep 1.54 for release
3f929dd 
Signed-off-by: Mark Phelps <[email protected]>
@markphelps
chore: update sdk/go
468cd33 
Signed-off-by: Mark Phelps <[email protected]>
@markphelps
chore: mod tidy
ef15950 
Signed-off-by: Mark Phelps <[email protected]>
@markphelps
Merge branch 'main' into release/1.54
b43e5a5 
* main:
  fix: update flag with no metadata (#3791)
  chore(deps): bump github.com/jackc/pgx/v5 from 5.7.1 to 5.7.2 (#3786)
  chore(deps): bump go.opentelemetry.io/otel/exporters/prometheus (#3785)
  chore(deps): bump github.com/go-git/go-git/v5 from 5.13.0 to 5.13.1 (#3783)
  chore(deps): bump golang.org/x/oauth2 from 0.24.0 to 0.25.0 (#3784)
  chore(deps): bump github.com/aws/aws-sdk-go-v2/service/ecrpublic (#3787)
  chore(deps): bump class-variance-authority from 0.7.0 to 0.7.1 in /ui (#3782)
  chore(deps-dev): bump @types/node from 18.19.65 to 18.19.69 in /ui (#3780)
  chore(deps): bump @uiw/codemirror-theme-tokyo-night in /ui (#3779)
  chore(deps): bump react-chartjs-2 from 5.2.0 to 5.3.0 in /ui (#3778)
  chore(deps): bump next in /examples/nextjs/pages-router (#3776)
  chore(deps): bump next in /examples/nextjs/app-router (#3777)
  chore: add release-notes to gitignore
  chore: fix release notes build
  chore: prep 1.54 for release (#3775)
@markphelps
chore: update changelog
41eda13 
Signed-off-by: Mark Phelps <[email protected]>
@markphelps
Merge branch 'main' of https://github.com/flipt-io/flipt
6ea664c 
* 'main' of https://github.com/flipt-io/flipt:
  chore(deps): bump @fortawesome/fontawesome-svg-core in /ui (#3802)
  chore(deps): bump github.com/spf13/afero from 1.11.0 to 1.12.0 (#3799)
  chore(deps): bump golang.org/x/net from 0.33.0 to 0.34.0 (#3800)
  chore(deps): bump go.opentelemetry.io/otel/exporters/zipkin (#3796)
  chore(deps): bump cloud.google.com/go/storage from 1.49.0 to 1.50.0 (#3798)
  chore(deps): bump go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp (#3797)
  chore(deps): bump alpine from 3.21.0 to 3.21.2 in /build (#3795)
  chore(deps): bump lucide-react from 0.468.0 to 0.471.0 in /ui (#3801)
  chore(deps-dev): bump @types/node from 18.19.69 to 18.19.70 in /ui (#3803)
  chore(deps): bump @fortawesome/free-brands-svg-icons in /ui (#3804)
  chore(deps): bump @codemirror/state from 6.5.0 to 6.5.1 in /ui (#3805)
  chore(deps): bump golang.org/x/tools from 0.28.0 to 0.29.0 in /_tools (#3793)
  chore(deps): bump google.golang.org/protobuf in /_tools (#3794)
  chore(deps-dev): bump eslint-plugin-react from 7.37.2 to 7.37.3 in /ui (#3781)
  chore: Add back release footer (#3792)
@markphelps
fix: issue with bucket prefix and gcp
542399f 
Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps requested a review from a team as a code owner January 15, 2025 22:09
@markphelps
chore: fix storage test
338cb74 
Signed-off-by: Mark Phelps <[email protected]>
@erka
Copy link
Contributor

erka commented Jan 15, 2025 •
edited
Loading

Prefix is a tricky thing. Prefix is not a folder as object storages doesn't have folders. At least in the past... AWS has changed this a bit recently. Prefixes production and production/ are not the same thing. Just for the record.

@codecov Codecov
Copy link

codecov bot commented Jan 15, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 33.33333% with 8 lines in your changes missing coverage. Please review.

Project coverage is 64.80%. Comparing base (31babb4) to head (910e6da).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/storage/fs/store/store.go 0.00% 7 Missing ⚠️
internal/storage/fs/object/store.go 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3808      +/-   ##
==========================================
- Coverage   64.84%   64.80%   -0.05%     
==========================================
  Files         171      171              
  Lines       17391    17387       -4     
==========================================
- Hits        11277    11267      -10     
- Misses       5418     5423       +5     
- Partials      696      697       +1
Flag Coverage Δ
unittests 64.80% <33.33%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@markphelps
Copy link
Collaborator Author

Prefix is a tricky thing. Prefix is not a folder as object storages doesn't have folders. At least in the past... AWS has changed this a bit recently. Prefixes production and production/ are not the same thing. Just for the record.

I think the go cloud blob.PrefixedBucket should handle these cases no? At the very least all the ITs now pass again. Mind giving it a review @erka ?

@markphelps markphelps requested a review from GeorgeMac January 16, 2025 16:20
@markphelps
chore: rm / adding to prefix
910e6da 
Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps requested a review from erka January 17, 2025 15:45
erka
erka approved these changes Jan 17, 2025
View reviewed changes
Copy link
Contributor

@erka erka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@markphelps markphelps added the automerge Used by Kodiak bot to automerge PRs label Jan 17, 2025
@kodiakhq kodiakhq bot merged commit cb278c8 into main Jan 17, 2025
38 of 40 checks passed
@kodiakhq kodiakhq bot deleted the fix-bucket-prefix branch January 17, 2025 15:50
markphelps added a commit that referenced this pull request Jan 17, 2025
@markphelps
Merge branch 'main' of https://github.com/flipt-io/flipt
b69cfb6 
* 'main' of https://github.com/flipt-io/flipt:
  fix: bucket prefix (#3808)
markphelps added a commit that referenced this pull request Jan 22, 2025
@markphelps @dependabot @erka
chore: Prep 1.54.2 (#3812)
7f23cda 
* chore: Add back release footer (#3792)

* chore: prep 1.54 for release

Signed-off-by: Mark Phelps <[email protected]>

* chore: update sdk/go

Signed-off-by: Mark Phelps <[email protected]>

* chore: mod tidy

Signed-off-by: Mark Phelps <[email protected]>

* chore: update changelog

Signed-off-by: Mark Phelps <[email protected]>

* chore: add back release notes footer

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>

* chore(deps-dev): bump eslint-plugin-react from 7.37.2 to 7.37.3 in /ui (#3781)

Bumps [eslint-plugin-react](https://github.com/jsx-eslint/eslint-plugin-react) from 7.37.2 to 7.37.3.
- [Release notes](https://github.com/jsx-eslint/eslint-plugin-react/releases)
- [Changelog](https://github.com/jsx-eslint/eslint-plugin-react/blob/master/CHANGELOG.md)
- [Commits](jsx-eslint/eslint-plugin-react@v7.37.2...v7.37.3)

---
updated-dependencies:
- dependency-name: eslint-plugin-react
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump google.golang.org/protobuf in /_tools (#3794)

Bumps google.golang.org/protobuf from 1.36.1 to 1.36.2.

---
updated-dependencies:
- dependency-name: google.golang.org/protobuf
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump golang.org/x/tools from 0.28.0 to 0.29.0 in /_tools (#3793)

Bumps [golang.org/x/tools](https://github.com/golang/tools) from 0.28.0 to 0.29.0.
- [Release notes](https://github.com/golang/tools/releases)
- [Commits](golang/tools@v0.28.0...v0.29.0)

---
updated-dependencies:
- dependency-name: golang.org/x/tools
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump @codemirror/state from 6.5.0 to 6.5.1 in /ui (#3805)

Bumps [@codemirror/state](https://github.com/codemirror/state) from 6.5.0 to 6.5.1.
- [Changelog](https://github.com/codemirror/state/blob/main/CHANGELOG.md)
- [Commits](codemirror/state@6.5.0...6.5.1)

---
updated-dependencies:
- dependency-name: "@codemirror/state"
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump @fortawesome/free-brands-svg-icons in /ui (#3804)

Bumps [@fortawesome/free-brands-svg-icons](https://github.com/FortAwesome/Font-Awesome) from 6.7.1 to 6.7.2.
- [Release notes](https://github.com/FortAwesome/Font-Awesome/releases)
- [Changelog](https://github.com/FortAwesome/Font-Awesome/blob/6.x/CHANGELOG.md)
- [Commits](FortAwesome/Font-Awesome@6.7.1...6.7.2)

---
updated-dependencies:
- dependency-name: "@fortawesome/free-brands-svg-icons"
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump @types/node from 18.19.69 to 18.19.70 in /ui (#3803)

Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 18.19.69 to 18.19.70.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)

---
updated-dependencies:
- dependency-name: "@types/node"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump lucide-react from 0.468.0 to 0.471.0 in /ui (#3801)

Bumps [lucide-react](https://github.com/lucide-icons/lucide/tree/HEAD/packages/lucide-react) from 0.468.0 to 0.471.0.
- [Release notes](https://github.com/lucide-icons/lucide/releases)
- [Commits](https://github.com/lucide-icons/lucide/commits/0.471.0/packages/lucide-react)

---
updated-dependencies:
- dependency-name: lucide-react
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump alpine from 3.21.0 to 3.21.2 in /build (#3795)

Bumps alpine from 3.21.0 to 3.21.2.

---
updated-dependencies:
- dependency-name: alpine
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp (#3797)

Bumps [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp](https://github.com/open-telemetry/opentelemetry-go) from 1.32.0 to 1.33.0.
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.32.0...v1.33.0)

---
updated-dependencies:
- dependency-name: go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump cloud.google.com/go/storage from 1.49.0 to 1.50.0 (#3798)

Bumps [cloud.google.com/go/storage](https://github.com/googleapis/google-cloud-go) from 1.49.0 to 1.50.0.
- [Release notes](https://github.com/googleapis/google-cloud-go/releases)
- [Changelog](https://github.com/googleapis/google-cloud-go/blob/main/CHANGES.md)
- [Commits](googleapis/google-cloud-go@spanner/v1.49.0...spanner/v1.50.0)

---
updated-dependencies:
- dependency-name: cloud.google.com/go/storage
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump go.opentelemetry.io/otel/exporters/zipkin (#3796)

Bumps [go.opentelemetry.io/otel/exporters/zipkin](https://github.com/open-telemetry/opentelemetry-go) from 1.31.0 to 1.33.0.
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.31.0...v1.33.0)

---
updated-dependencies:
- dependency-name: go.opentelemetry.io/otel/exporters/zipkin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump golang.org/x/net from 0.33.0 to 0.34.0 (#3800)

Bumps [golang.org/x/net](https://github.com/golang/net) from 0.33.0 to 0.34.0.
- [Commits](golang/net@v0.33.0...v0.34.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump github.com/spf13/afero from 1.11.0 to 1.12.0 (#3799)

Bumps [github.com/spf13/afero](https://github.com/spf13/afero) from 1.11.0 to 1.12.0.
- [Release notes](https://github.com/spf13/afero/releases)
- [Commits](spf13/afero@v1.11.0...v1.12.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/afero
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump @fortawesome/fontawesome-svg-core in /ui (#3802)

Bumps [@fortawesome/fontawesome-svg-core](https://github.com/FortAwesome/Font-Awesome) from 6.7.1 to 6.7.2.
- [Release notes](https://github.com/FortAwesome/Font-Awesome/releases)
- [Changelog](https://github.com/FortAwesome/Font-Awesome/blob/6.x/CHANGELOG.md)
- [Commits](FortAwesome/Font-Awesome@6.7.1...6.7.2)

---
updated-dependencies:
- dependency-name: "@fortawesome/fontawesome-svg-core"
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: bucket prefix (#3808)

* chore: prep 1.54 for release

Signed-off-by: Mark Phelps <[email protected]>

* chore: update sdk/go

Signed-off-by: Mark Phelps <[email protected]>

* chore: mod tidy

Signed-off-by: Mark Phelps <[email protected]>

* chore: update changelog

Signed-off-by: Mark Phelps <[email protected]>

* fix: issue with bucket prefix and gcp

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix storage test

Signed-off-by: Mark Phelps <[email protected]>

* chore: only add / to prefix if prefix exists

Signed-off-by: Mark Phelps <[email protected]>

* chore: rm / adding to prefix

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>

* chore: Rm cloud banners (#3810)

* chore: prep 1.54 for release

Signed-off-by: Mark Phelps <[email protected]>

* chore: update sdk/go

Signed-off-by: Mark Phelps <[email protected]>

* chore: mod tidy

Signed-off-by: Mark Phelps <[email protected]>

* chore: update changelog

Signed-off-by: Mark Phelps <[email protected]>

* chore: rm flipt cloud mentions :(

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>

* chore(deps): go work sync (#3809)

* chore: prep 1.54.2 for release

Signed-off-by: Mark Phelps <[email protected]>

* chore: update changelog

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Roman Dmytrenko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erka erka erka approved these changes

@GeorgeMac GeorgeMac Awaiting requested review from GeorgeMac

Assignees
No one assigned
Labels
automerge Used by Kodiak bot to automerge PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Server Unable to Locate .flipt.yml and features.yml in GCloud Bucket
2 participants
@markphelps @erka