Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git-clone Doesn't support authenticated requests to git repos out of the box #201

Closed
ghost opened this issue Feb 27, 2020 · 8 comments
Closed
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@ghost
Copy link

ghost commented Feb 27, 2020

Expected Behavior

The git-clone task provided by the catalog should have a clear path for supporting authenticated requests to repositories. The Git Resource that the git-clone task is taken from relied on the creds-init binary to ensure that the .ssh folder was populated correctly before its own steps ran. See the code for creds-init here: https://github.com/tektoncd/pipeline/blob/master/cmd/creds-init/main.go and for the git creds builder here: https://github.com/tektoncd/pipeline/blob/9b1ec33f77d24afbeac27ea0d0b4aeeac65cdd2b/pkg/credentials/gitcreds/creds.go

Actual Behavior

The git-clone task doesn't support credentialed actions yet. Yes it does, but the user journey could be much improved.

@ghost
Copy link
Author

ghost commented Feb 27, 2020

/assign

@ghost
Copy link
Author

ghost commented Feb 27, 2020

Addendum: It looks like the creds-init binary / container does run if a service account is attached. See code in pkg/pod/pod.go here: https://github.com/tektoncd/pipeline/blob/master/pkg/pod/pod.go#L89-L95 and the actual creds init package here: https://github.com/tektoncd/pipeline/blob/9b1ec33f77d24afbeac27ea0d0b4aeeac65cdd2b/pkg/pod/creds_init.go#L39

So I think this should work with the git-clone task. But I haven't tested it yet and need to confirm this. It works.

Either way, I still kinda feel like this kind of "implicit" auth that happens under the covers is really unintuitive. I'd much prefer some kind of credential task that does this work explicitly as part of a pipeline and writes to a shared Workspace OR that we simply require the user to provide a Workspace directly from a Secret.

Edit: I've tested that the creds-init flow documented in our auth.md does work for the git-clone task.

@vdemeester
Copy link
Member

/kind feature

I do agree on the unintuitiveness of the implicit auth

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 28, 2020
@jlpettersson
Copy link
Member

jlpettersson commented May 10, 2020

I hadn't seen this issue before. But I added a draft of a git-clone-ssh task with very explicit SSH authentication, #309.

Either way, I still kinda feel like this kind of "implicit" auth that happens under the covers is really unintuitive.

I totally agree.

I'd much prefer some kind of credential task that does this work explicitly as part of a pipeline

Every task that "reach out", e.g. doing any kind of network request to POST data or GET data need any kind of authentication. Authentication can be done with either a serviceAccount (best) or password/credential/shared-key. This is a secret or token specific (and private) to a step. How this is loaded by the "command" in the step depends very much on the implementation.

My idea now:
Tasks that could have multiple types of authentication, could potentially exists in different variants, e.g. git-ssh and git-token. For composability those should have the same contract/interface so that the implementation could be swapped out to another.
In Tekton Catalog:

git-ssh/git-clone.yaml
git-ssh/README.md
git-token/git-clone.yaml
git-token/README.md

If git-ssh and git-token implement the same contract/interface, that means; they both have the same name: git-clone, they have the same results: and all required params: must exists in both, (but optional could differ?)

This means that a Pipeline with PipelineTaskBinding look the same, e.g. taskRef: git-clone and params. But what implementation e.g. git-ssh or git-token that is deployed in the cluster can differ, and be swapped.

(Hm, could this be organized as a kustomize directory structure in Catalog? a common base, and two different overlays; overlay/git-ssh and overlay/git-token. I don't know)

@ghost
Copy link
Author

ghost commented May 11, 2020

Tasks that could have multiple types of authentication, could potentially exists in different variants, e.g. git-ssh and git-token. For composability those should have the same contract/interface so that the implementation could be swapped out to another.

Do you think it's strictly better to have separate Tasks based on auth or do you think that given Tekton's current state this is the best solution available? I am still of the mind that having a good system for multiple types of credentials to be supported in a single Task is useful. Do you think that's true or you'd prefer to always keep tasks with different auth mechanisms separated?

@jlpettersson
Copy link
Member

jlpettersson commented May 11, 2020

Do you think it's strictly better to have separate Tasks based on auth or do you think that given Tekton's current state this is the best solution available? I am still of the mind that having a good system for multiple types of credentials to be supported in a single Task is useful. Do you think that's true or you'd prefer to always keep tasks with different auth mechanisms separated?

I agree with you. I am not sure that it is strictly better to have different tasks. Managing a contract/interface with multiple tasks also has a management cost.

Another case for contract/interface would be: build-image-from-Dockerfile and both buildah and kaniko could implement that case.

But the most easy would be to have a single git-clone task as you say @sbwsg , that supports both methonds. I started to PoC on that late yesterday:

This part is at least possible: (optional secrets for both types of auth - or none)

  volumes:
  - name: git-auth
    projected:
      defaultMode: 0400
      sources:
      - secret:
          name: $(params.ssh-known-hosts-secret)
          optional: true
      - secret:
          name: $(params.ssh-private-key-secret)
          optional: true
      - secret:
          name: $(params.git-credentials-file-secret)
          optional: true

@ghost
Copy link
Author

ghost commented May 12, 2020

Wow, I had no idea that optional was a supported field in projected volumes! Even looking at the design doc linked from the official website - the optional field is mentioned in the spec but given absolutely zero description anywhere else.

This gives me a lot to think about, especially now that you've added variable interpolation for those field names. Very interesting...

@ghost
Copy link
Author

ghost commented Jul 27, 2020

Given that the git-clone task does support creds-init style authentication and that we now have a dedicated thread of work related to improving credentials support, I'm going to call this issue closed.

@ghost ghost closed this as completed Jul 27, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants