Skip to content

Conversation

@cydu-cloud
Copy link
Contributor

GIT_ASKPASS_URL callback support external HTTP Service to provide password and username to access Git Repo.

See https://github.com/cydu-cloud/git-askpass-gce-node as end to end test which enables git-sync access GCP Cloud Source Repo with service account's oauth bear token.

See https://git-scm.com/docs/gitcredentials for more about the GIT_ASKPASS protocols.

It specifies a HTTP URL which will return username&password which will
be used to authenticate access to the git repo.

This is mainly used for git repo accecpt dynamic password (for example
oauth bare token). Because the dynamic password might expire very soon,
so it's added to the main syncRepo loop.

Typical usage case is work with a sidecar called gce-node-auth on GKE,
it uses the GCE service account's oauth token as password to access
Cloud Source Repo.

Please see the repo below for how it worked.
https://github.com/cydu-cloud/gce-node-auth/blob/master/git-sync-with-gce-node-auth.yaml
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Dec 17, 2019
@k8s-ci-robot
Copy link
Contributor

Welcome @cydu-cloud!

It looks like this is your first PR to kubernetes/git-sync 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/git-sync has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 17, 2019
@cydu-cloud
Copy link
Contributor Author

I signed it

@cydu-cloud
Copy link
Contributor Author

I signed it

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Dec 17, 2019
@cydu-cloud
Copy link
Contributor Author

/assign @thockin


The recommended way is the ASKPASS Service running within the same pod as git-sync.

See <https://github.com/cydu-cloud/git-askpass-gce-node> as a full example which use GCE Node Service Account credential to access Google Cloud Source Repo.

Choose a reason for hiding this comment

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

We should find a better place to put this.. maybe GoogleCloudPlatform's repo?

Copy link
Member

Choose a reason for hiding this comment

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

Can we cook down an e2e test that can be set up and run locally? E.g. spin up a git server and an HTTP server, tell git-sync to pull the password from the HTTP server.

test_e2e.sh should cover this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the e2e test, it depends on askpass_git test I just created in another PR, don't want the PR grow into too big, can I add it as follow up PR? This won't hurt any existing functionality even the callbalk is broken.

For GoogleCloudPlatform's repo, it might take a while to setup, need to go through some process.

Copy link
Member

Choose a reason for hiding this comment

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

I don't want to e2e against GCP. I want to set up a local git repo that demands user/pass and a local HTTP server which serves the content. I am working on a more complex SSH example - will link it here, hopefully helps you with infra

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For "local git repo that demands user/pass", trying to use askpass_git as I did for user/pass e2e test, the git credential fill is exactly what git do when demands user/pass, so the simulation should be very good enough for the test here.
Removed the reference docs to gcp. It's purely local e2e test right now.

Thanks for the SSH example, I can not image any better way to do it for SSH, but for user/pass, I personally prefer askpass_git a bit more since it's light and easier to debug/maintain.

Make sense?

Sorry for the growing PR, I just manually merged the askpass_git from #217.

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Overall this looks great - is an e2e test possible?


The recommended way is the ASKPASS Service running within the same pod as git-sync.

See <https://github.com/cydu-cloud/git-askpass-gce-node> as a full example which use GCE Node Service Account credential to access Google Cloud Source Repo.
Copy link
Member

Choose a reason for hiding this comment

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

I don't want to e2e against GCP. I want to set up a local git repo that demands user/pass and a local HTTP server which serves the content. I am working on a more complex SSH example - will link it here, hopefully helps you with infra

@thockin
Copy link
Member

thockin commented Dec 19, 2019

See #218 for e2e infra ideas (last commit)

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Also, please squash commits

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Thanks!

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 20, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thockin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2019
@k8s-ci-robot k8s-ci-robot merged commit d8928aa into kubernetes:master Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants