Allow for fetching packages from remote git repositories#255
Allow for fetching packages from remote git repositories#255nimakaviani merged 3 commits intocnoe-io:mainfrom
Conversation
d289b8a to
b64b627
Compare
|
Seeing some weirdness around re-cloning repositories with depth 1. Seems like it's a bug fixed in a release later than what we are using. I'll need to update it. |
Signed-off-by: Manabu McCloskey <manabu.mccloskey@gmail.com>
nimakaviani
left a comment
There was a problem hiding this comment.
Thanks for this left a few comments.
I think we should also update the README to go with it. Can you please make the necessary changes?
| hash, push, err := addAllAndCommit(repo.Spec.Source.Path, tgtRepository) | ||
| if err != nil { | ||
| return fmt.Errorf("pushing to git: %w", err) | ||
| return fmt.Errorf("add and commit %w", err) | ||
| } | ||
|
|
||
| if push { | ||
| remoteUrl, err := util.FirstRemoteURL(tgtRepository) | ||
| if err != nil { | ||
| return fmt.Errorf("getting remote url %w", err) | ||
| } | ||
|
|
||
| logger.V(1).Info("pushing to remote url %s", remoteUrl) | ||
| err = pushToRemote(ctx, tgtRepository, creds) | ||
| if err != nil { | ||
| return fmt.Errorf("pushing to git: %w", err) | ||
| } | ||
|
|
||
| repo.Status.LatestCommit.Hash = hash.String() | ||
| return nil | ||
| } | ||
|
|
||
| repo.Status.LatestCommit.Hash = commit.String() | ||
| repo.Status.LatestCommit.Hash = hash.String() | ||
| return nil |
There was a problem hiding this comment.
there is some redundancy between reconcileLocalRepoContent and reconcileRemoteRepoContent. Can we pull them into one function for clarity?
There was a problem hiding this comment.
It does looks similar but I don't think introducing another function would save us many lines. Maybe like 3 lines. I think it's pretty clear what these functions are doing right now and I'd rather keep it that way.
Signed-off-by: Manabu McCloskey <manabu.mccloskey@gmail.com>
|
Updated Readme and addressed review comments. @nimakaviani |
README.md
Outdated
| Idpbuilder supports specifying custom packages using the flag `--package-dir` flag. | ||
| This flag expects a directory (local or remote) containing ArgoCD application files. | ||
| In case of a remote directory, it must be a directory in a git repository, | ||
| and the URL format must be a [kustommize remote URL format](https://github.com/kubernetes-sigs/kustomize/blob/master/examples/remoteBuild.md). |
There was a problem hiding this comment.
Typo error: kustommize => kustomize
Why do you force the users to use a kustomize URL instead of HTTPS or SSH git urls ? What is the added value (if there is) to use kustomize url here ?
There was a problem hiding this comment.
Gosh I really should configure my code spell plugin my spelling isn't good.
It actually supports git SSH format. The problem with just plain HTTPS format is that there's no way for us to know where the repository URL ends. For GitHub, it's almost always org-name/repo-name, but that's not the case for other git providers. This is the main reason for me to use the kustomize format.
nimakaviani
left a comment
There was a problem hiding this comment.
given that we have #264 to add a new flag to handle package references, I am going to approve and merge this, so we can unblock the rest of the work.
This allows for fetching packages from remote git repositories.
The URL format follows kustomize remote url format.
fixes: #184
example:
This should also reduce the number of git clone operations against local and remote repositories.