feat(oci): better integration OCI storage with AWS ECR

[erka]

resolves #2938

[erka]

I would like to be advised about awsECRCredential. How could it be better? Tested or refactored?

[erka]

@thepabloaguilar this is a better place to discuss. I opened that PR in wrong location.

[codecov]

Codecov Report

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

Project coverage is 72.49%. Comparing base (f997fb9) to head (0c01148).
Report is 209 commits behind head on main.

Files	Patch %	Lines
internal/oci/ecr/mock_client.go	58.62%	5 Missing and 7 partials ⚠️
internal/oci/ecr/ecr.go	85.18%	3 Missing and 1 partial ⚠️
internal/oci/file.go	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2941      +/-   ##
==========================================
+ Coverage   70.78%   72.49%   +1.70%     
==========================================
  Files          91       94       +3     
  Lines        8729     7140    -1589     
==========================================
- Hits         6179     5176    -1003     
+ Misses       2165     1575     -590     
- Partials      385      389       +4     

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

[GeorgeMac]

Change is looking great to me. One suggestion so far on validating the OCI auth type. Otherwise though, this is looking great.

Update: for some reason GitHub did not add my inline comment...
Let me add that again...

[erka]

@GeorgeMac I need some help. I have an issue with configuration default values in flipt bundle and integration.

flipt/build/testing/integration.go

Lines 498 to 510 in 4749907

	if _, err := flipt.
	WithDirectory("/tmp/testdata", base.Directory(singleRevisionTestdataDir)).
	WithWorkdir("/tmp/testdata").
	WithServiceBinding("zot", zot.AsService()).
	WithEnvVariable("FLIPT_STORAGE_OCI_AUTHENTICATION_USERNAME", "username").
	WithEnvVariable("FLIPT_STORAGE_OCI_AUTHENTICATION_PASSWORD", "password").
	WithExec([]string{"/flipt", "bundle", "build", "readonly:latest"}).
	WithExec([]string{"/flipt", "bundle", "push", "readonly:latest", "http://zot:5000/readonly:latest"}).
	Sync(ctx); err != nil {
	return func() error {
	return err
	}
	}

There is no FLIPT_STORAGE_TYPE=oci definition. As result, default values from internal/config/storage.go#L72 are not set as there is no oci expected. What would you recommend to do? Should I add extra defaults in bundle.go? Or should I add FLIPT_STORAGE_TYPE=oci in integration? Any other option?

[GeorgeMac]

@GeorgeMac I need some help. I have an issue with configuration default values in flipt bundle and integration.

flipt/build/testing/integration.go

Lines 498 to 510 in 4749907

	if _, err := flipt.
	WithDirectory("/tmp/testdata", base.Directory(singleRevisionTestdataDir)).
	WithWorkdir("/tmp/testdata").
	WithServiceBinding("zot", zot.AsService()).
	WithEnvVariable("FLIPT_STORAGE_OCI_AUTHENTICATION_USERNAME", "username").
	WithEnvVariable("FLIPT_STORAGE_OCI_AUTHENTICATION_PASSWORD", "password").
	WithExec([]string{"/flipt", "bundle", "build", "readonly:latest"}).
	WithExec([]string{"/flipt", "bundle", "push", "readonly:latest", "http://zot:5000/readonly:latest"}).
	Sync(ctx); err != nil {
	return func() error {
	return err
	}
	}

There is no FLIPT_STORAGE_TYPE=oci definition. As result, default values from internal/config/storage.go#L72 are not set as there is no oci expected. What would you recommend to do? Should I add extra defaults in bundle.go? Or should I add FLIPT_STORAGE_TYPE=oci in integration? Any other option?

Ahh I see what you're saying, we don't set it on this branch of the dagger suite, as flipt bundle essentially implies storage type OCI. In hindsight, perhaps sharing the flipt config here for the bundle command wasn't a great idea.

I think, ideally, you wouldn't need to have FLIPT_STORAGE_TYPE=oci set, for the flipt bundle to have sane defaults. So I think your suggestion to them in bundle.go makes most sense, if that is possible / not a pain to do.

[erka]

I think, ideally, you wouldn't need to have FLIPT_STORAGE_TYPE=oci set, for the flipt bundle to have sane defaults. So I think your suggestion to them in bundle.go makes most sense, if that is possible / not a pain to do.

@GeorgeMac I experimented a bit with options and defining defaults in bundle.go is the way. Thank you for your advice.

[GeorgeMac]

👏 appreciate it as always @erka Looks great.

Does this suit your needs @thepabloaguilar ? If so, we can get it in and I think we were planning a new minor for either today or monday. Good timing 👏

[thepabloaguilar]

Yeap @GeorgeMac, it seems to be!