feat(registry): support AssumeRoleWithWebIdentity for S3

[davidspek]

Comprehensive Summary of your change

This PR adds patches to the building of the Harbor registry image that update the AWS SDK and fix how the AWS S3 client is created so that it can use AssumeRoleWithWebIdentity through IRSA.

The upstream PRs used in the patches are distribution/distribution#3921 that change the way the client is created and distribution/distribution#3599 that updates the AWS SDK. I have built the image that results from this PR and verified I can push new images to Harbor while using IRSA. For testing the image davidspek/registry-photon:v2.8.0-irsa-0.1.4 can be used.

Issue being fixed

Fixes #16490 and #12888

Please indicate you've done the following:

• Well Written Title and Summary of the PR
• Label the PR as needed. "release-note/new-feature"
• Accepted the DCO. Commits without the DCO will delay acceptance.
• Made sure tests are passing and test coverage is added if needed.
• Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1204651419195138

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.39%. Comparing base (fb52fdb) to head (2e24e09).
Report is 388 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #18686      +/-   ##
==========================================
- Coverage   67.39%   67.39%   -0.01%     
==========================================
  Files         981      981              
  Lines      107194   107194              
  Branches     2698     2698              
==========================================
- Hits        72246    72241       -5     
- Misses      31065    31069       +4     
- Partials     3883     3884       +1     

Flag	Coverage Δ
unittests	67.39% <ø> (-0.01%)	⬇️

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

see 8 files with indirect coverage changes

[davidspek]

Friendly ping @Vad1mo @wy65701436 @MinerYang

[Vad1mo]

@davidspek can you demo the setup and configuration in the next community meeting, we can use recording for documentation.

[davidspek]

@Vad1mo Sorry for the late response. When is the next community meeting?

[daemon-ian]

Hi @Vad1mo @wy65701436 @MinerYang - are we clear what the next steps are for getting this merged? It feels like we're really close to getting this long standing issue sorted. Thanks @davidspek for putting this in.

[davidspek]

@OrlinVasilev maybe you can also weigh in here.

[OrlinVasilev]

@wy65701436 can you check please!

[OrlinVasilev]

@davidspek https://goharbor.io/community/ here is the community page, tomorrow(28th June) we have community meeting here is the calendar link https://lists.cncf.io/g/harbor-users/viewevent?repeatid=51606&eventid=1944638&calstart=2023-06-28 also please add that to the agenda here : https://github.com/goharbor/community/wiki/Harbor-Community-Meetings#june-28-2023

[davidspek]

@OrlinVasilev Thanks for the info. I've added the meeting to my calendar and added discussing this PR to the agenda.

[OrlinVasilev]

@davidspek Thank you :)) see you later :)

[wy65701436]

holding to discuss

[wy65701436]

I'm against this change because we do not want to maintain any forked or modified codebase that deviates from the original one. Instead, we should align with the upstream release cadence and pick up the next release that contains the two PRs. While we currently only have two or three backports that do not have any conflicts, we need to consider the long-term maintenance of our codebase.

Furthermore, the redis patch is critical for users who want to enable redis high availability in sentinel. We only have one patch, which is not likely to conflict with other changes.

[peterellisjones]

Without this change Harbor is not suitable for most enterprise users using EKS.

1. Multiple registry replicas are required for harbor to be HA
2. With multiple replicas, the registry deployment only works with an external blobstore, not filesystem storage. On AWS, this means S3.
3. Without this PR, Harbor can only authenticate with S3 via static credentials (access key), not IRSA
4. No enterprise security team wants static S3 credentials floating around when IRSA exists, and they are generally prohibited.

[OrlinVasilev]

Hi @peterellisjones can you join us later today so we can all discuss that, @wy65701436 can you please also join :)

[mhrabovcin]

I would be in favour of merging this PR as there is no clear timeline in the upstream registry project when there will be a supported release (v3 or patched v2.x) that can use dynamic AWS credentials.

[OrlinVasilev]

As discussed yesterday @wy65701436 and @davidspek will join forces to try reaching out to distribution maintainers to merge the fixes so we can have something tested and not customized on our side! you can find the recording here :) https://youtu.be/aMkzKf1vmbQ

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

[dkulchinsky]

not stale

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

[ichasco-heytrade]

not stale

[OrlinVasilev]

@wy65701436 and @davidspek any progress on that?

[peterellisjones]

Would love to see some progress on this. IMHO without support for assume role Harbor is not really an appropriate product for enterprise users

[OrlinVasilev]

@peterellisjones absolutely, that's the reason I'm trying to prioritize that :) @msha01 can you put that in the log please!

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

[Vad1mo]

We should close or update this PR.

Harbor depends on upstream docker distribution, and we are not hard forking docker distribution. We only apply PRs that are in the main branch, but where a release is not published yet.

It looks like that all of the upstream PRs have been closed

• [release/2.8] vendor: github.com/aws/aws-sdk-go v1.43.16 distribution/distribution#3599
• [release/2.8] feat: allow for IRSA with s3 distribution/distribution#3921

The consequence here is A to update this PR or B close this PR.

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.