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.

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

feat: make oci manifest version configurable #2908

Merged
merged 5 commits into from
Mar 27, 2024

Conversation

thepabloaguilar
Copy link
Contributor

Hi! I'm opening this PR to fix that issue with AWS ECR, when using the OCI Manifest Version 1.1 I receive an error from AWS but when setting to version 1.0 everything works as expected!

So I make it configurable keeping the current using version as default! I searched a bit the code to see if flipt could have any impact on letting the users choose between versions and the answer is no, flipt won't use the newly added field (on 1.1 version) subject! If you want to compare: v1.0.2 abd v1.1.0

Other people are having the same impact:

Fixes #2907

@thepabloaguilar thepabloaguilar requested a review from a team as a code owner March 27, 2024 04:21
@thepabloaguilar
Copy link
Contributor Author

If you accept this PR would be possible to make a minor release, please? 😄

Copy link

codecov bot commented Mar 27, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 72.27%. Comparing base (f997fb9) to head (97744ac).
Report is 180 commits behind head on main.

Files Patch % Lines
internal/oci/file.go 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2908      +/-   ##
==========================================
+ Coverage   70.78%   72.27%   +1.49%     
==========================================
  Files          91       92       +1     
  Lines        8729     7146    -1583     
==========================================
- Hits         6179     5165    -1014     
+ Misses       2165     1597     -568     
+ Partials      385      384       -1     

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

config/flipt.schema.cue Outdated Show resolved Hide resolved
config/flipt.schema.json Outdated Show resolved Hide resolved
Copy link
Member

@GeorgeMac GeorgeMac left a comment

Choose a reason for hiding this comment

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

Thanks for discovering and fixing this @thepabloaguilar !

Just a couple comments on here, otherwise, it looks great.
We can get this in a patch release for you 👍

@thepabloaguilar
Copy link
Contributor Author

Done @GeorgeMac!

@markphelps markphelps added the needs docs Requires documentation updates label Mar 27, 2024
Copy link
Collaborator

@markphelps markphelps 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 to me! thanks @thepabloaguilar !

Im guessing we should document this as well in our configuration docs?

@thepabloaguilar
Copy link
Contributor Author

Sure @markphelps! I'd document putting a note or smth saying the problem with AWS and it can happen for other providers.

And I think we forgot to document the allowed_teams on here: Configuration - Overview - Authentication Methods: GitHub

Copy link
Member

@GeorgeMac GeorgeMac left a comment

Choose a reason for hiding this comment

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

Solid!

@markphelps markphelps added the automerge Used by Kodiak bot to automerge PRs label Mar 27, 2024
@kodiakhq kodiakhq bot merged commit b4bb5e1 into flipt-io:main Mar 27, 2024
26 of 27 checks passed
@markphelps
Copy link
Collaborator

Sure @markphelps! I'd document putting a note or smth saying the problem with AWS and it can happen for other providers.

And I think we forgot to document the allowed_teams on here: Configuration - Overview - Authentication Methods: GitHub

@thepabloaguilar PR here flipt-io/docs#197

@thepabloaguilar thepabloaguilar deleted the issue-2907 branch March 27, 2024 14:38
@GeorgeMac
Copy link
Member

@thepabloaguilar Flipt v1.39.1 is out now 👍

markphelps added a commit that referenced this pull request Mar 29, 2024
* 'main' of https://github.com/flipt-io/flipt:
  feat: make oci manifest version configurable (#2908)
markphelps added a commit that referenced this pull request Mar 29, 2024
* main:
  docs: add tegorov as a contributor for code (#2921)
  Fix namespace conflict (#2920)
  docs: add proxeter as a contributor for code (#2913)
  Fix IsNotOneOf operator (#2912)
  chore: release 1.39.1 (#2911)
  feat: make oci manifest version configurable (#2908)
  chore: prep for 1.39 release (#2894)
@markphelps markphelps removed the needs docs Requires documentation updates label May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Used by Kodiak bot to automerge PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Error uploading OCI artifact to AWS ECR
3 participants