Add support for git credentials for ACM Subscriptions.

[adelton]

Description

Add support for git credentials for ACM Subscriptions.

Since the GitOps pipeline can file pull requests with container image changes in authenticated git repositories, ACM should be able to deploy applications from those repositories as well.

I tested that a different access token can be used (only for reading).

How Has This Been Tested?

Manually.

Merge criteria:

• [x] The commits are squashed in a cohesive manner and have meaningful messages.
• [ ] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• [ ] The developer has manually tested the changes and verified that the changes work

[adelton]

I couldn't find a way to make the replacements optional, only to run when the Secret actually exists. I'm hitting nothing selected by Secret....

[adelton]

Rebased on main.

[adelton]

Hello @grdryn, folks, this seemed to be needed to make use of private GitHub repositories possible. Unless we have some other approach in the works to handle those, I'd appreciate a review.

[adelton]

@grdryn So instead of that oc apply -k acm/registration/ we now have in https://github.com/opendatahub-io/ai-edge#deploy-the-applications-to-remote-clusters, what would the oc sequence be?

[grdryn]

@adelton good question, I'm not 100% sure what's best :thinking:

Maybe it could be split in two, to be either oc apply -k acm/registration/near-edge or oc apply -k acm/registration/near-edge-private or something?

Alternatively, if we just assume that repos are always private and need authentication and make that the default, it'll also still work for public repos, right? That might be the way to keep the documentation as simple as possible, and not need to have sections to uncomment in different scenarios? WDYT?

[adelton]

The idea that the extra secrets could be there uncommented because they wouldn't harm access to public git repositories does not seem to work.

When I just uncomment the bits in acm/registration/near-edge/overlays/bike-rental-app/kustomization.yaml, keeping the user and accessToken values there

secretGenerator:
- name: git-credentials
  literals:
  - user=username
  - accessToken=access_token

I get failed Subscription with

Cluster deploy status local-cluster: PropagationFailed Error: failed to initialize Git connection, err: Failed to clone git: https://github.com/opendatahub-io/ai-edge err: authentication required

When I set the values to empty with

secretGenerator:
- name: git-credentials
  literals:
  - user=
  - accessToken=

or remove them altogether with

secretGenerator:
- name: git-credentials
  literals:

I get

Cluster deploy status local-cluster: PropagationFailed Error: failed to initialize Git connection, err: sshKey (and optionally passphrase) or user and accressToken need to be specified in the channel secret

So it looks like when the spec.secretRef.name is set, the connection is considered authenticated and correct credentials need to be specified.

[adelton]

I'm not fond of that acm/registration/near-edge vs acm/registration/near-edge-private because whether the git repo should be authenticated or not is a per application (well, per Channel) thing, not a "this ACM setup" thing.

[adelton]

I also tried to move both the secretGenerator and the replacements entry to separate kustomization.yaml in a separate directory and tried to include it with a simple commented out / uncommented entry in resources but that also does not work without duplicating most everything in that new kustomization.yaml.

We might need some serious Kustomization-fu if we want to avoid the comments approach I have there now.

[grdryn]

The idea that the extra secrets could be there uncommented because they wouldn't harm access to public git repositories does not seem to work.

When I just uncomment the bits in acm/registration/near-edge/overlays/bike-rental-app/kustomization.yaml, keeping the user and accessToken values there

secretGenerator:
- name: git-credentials
  literals:
  - user=username
  - accessToken=access_token

I get failed Subscription with

Cluster deploy status local-cluster: PropagationFailed Error: failed to initialize Git connection, err: Failed to clone git: https://github.com/opendatahub-io/ai-edge err: authentication required

What I was suggesting for that was to require actual/valid values rater than dummy ones (my understanding is that you used dummy values here ("username" and "access_token")), but I might have misunderstood.

I also tried to move both the secretGenerator and the replacements entry to separate kustomization.yaml in a separate directory and tried to include it with a simple commented out / uncommented entry in resources but that also does not work without duplicating most everything in that new kustomization.yaml

Huh, that's disappointing, I was hoping it would be a straightforward thing, and that the second kustomization.yaml would be minimal.

We might need some serious Kustomization-fu if we want to avoid the comments approach I have there now.

Yeah, while kustomize is convenient because of how kubectl/oc can process it (and ACM requires it); the more we use it and brush up against its limitations, the more I miss the power of less integrated things like Dhall.

I'll approve this PR for now, so that you can merge this version if you want. :+1:

[adelton]

This change makes it possible to use either public and private repos, defaulting to the assumption that they'll be public, requiring a change for private ones. Would there be any value in switching to defaulting to private instead?

The current state of the README.mds allows you to setup the PoC without changing virtually nothing -- we default to the https://github.com/opendatahub-io/ai-edge repo, you don't have to even create a fork, you can run with the upstream (this repo) and it works.

The minute you switch to private repos, you force people to fiddle with accounts, forks, access tokens, ... I don't think that's the user friendly default.

[adelton]

Thanks for the ack, merging.