Implement the github Resource type about pipeline HOT 8 CLOSED

@pivotal-nader-ziada this is the issue I was mentioning re. the github resource!

from pipeline.

Sounds good

/assign @pivotal-nader-ziada

from pipeline.

If it is possible to share any of this logic with knative/build, that is great! knative/build-pipeline is likely going to depend on knative/build anyway

It is better to pull the common parts we need to the pkg repo and use it from there in both build and build-pipeline, or another common repo?

TaskRun currently doesn't seem to have any notion of the serviceAccount to use (e.g. in this example), the serviceAccount is currently specified only in the PipelineParams

The resource itself has the ServiceAccount and the TaskRun refers to the resource

(Open to redesign: instead of repeating all of the resources and credentials in the TaskRun, TaskRun could have a references to the Resource and PipelineParam CRDs. One downside tho is that if you look at a TaskRun, you won't know for sure what values were used for these, since they could have been subsequently updated.)

I think we have a piece of that already in TaskRun using ResourceRef to link back to the resource and also having resourceVersion to indicate which version this specific task ran with. Isn't the taskRun result going to be saved as part of the pipelineRun?

from pipeline.

It is better to pull the common parts we need to the pkg repo and use it from there in both build and build-pipeline, or another common repo?

If you prefer that approach we can do it, but I prefer keeping the build related logic in build, since I think that's a reasonable grouping of logic.

One of the downsides of the pkg repo is that it isn't well factored; it's a grab bag of random stuff that seems to be useful across repos. build at least is limited to build related stuff so I kind of like that.

The resource itself has the ServiceAccount and the TaskRun refers to the resource

Oh interesting, I totally missed that the resource has a ServiceAccount. We had been assuming/hoping we could get away with one top level ServiceAccount for all actions a pipeline does, but it sounds like you'd rather have the flexibility of using different ServiceAccounts for different Resources?

I think we have a piece of that already in TaskRun using ResourceRef to link back to the resource and also having resourceVersion to indicate which version this specific task ran with. Isn't the taskRun result going to be saved as part of the pipelineRun?

Yep you are totally right, that's another thing (the fact that TaskRuns now have references to Resources) I missed!

The TaskRun will be saved into the run results - if we want to keep this as a ref, we could save a snapshot of the Resource as well - but if that's the way we want to go, I think we should do the same thing with other values such as the results which actually come from the PipelineParams. Maybe this is a better way to go so folks creating TaskRuns manually don't have to repeat as much info?

from pipeline.

If you prefer that approach we can do it, but I prefer keeping the build related logic in build, since I think that's a reasonable grouping of logic.

I'm fine with keeping it in build and depending on it there, I think we will have this dependency anyways. If we later decide to make the dependency less coupled, we can pull out the common things even in another common repo.

Oh interesting, I totally missed that the resource has a ServiceAccount. We had been assuming/hoping we could get away with one top level ServiceAccount for all actions a pipeline does, but it sounds like you'd rather have the flexibility of using different ServiceAccounts for different Resources?

yeah in case you have different accounts to get from buckets vs from github, I have seen this use case before, what do you think?

The TaskRun will be saved into the run results - if we want to keep this as a ref, we could save a snapshot of the Resource as well - but if that's the way we want to go, I think we should do the same thing with other values such as the results which actually come from the PipelineParams. Maybe this is a better way to go so folks creating TaskRuns manually don't have to repeat as much info?

I like the approach of saving the snapshot with the ref so everything is in one place

from pipeline.

. If we later decide to make the dependency less coupled, we can pull out the common things even in another common repo.

Sounds good to me!

yeah in case you have different accounts to get from buckets vs from github, I have seen this use case before, what do you think?

I think you're right, especially if you've seen it before. It'll be more flexible anyway!

I like the approach of saving the snapshot with the ref so everything is in one place

kk then let's assume that! In that case the PipelineRun would have references to PipelineParams + Resources, and TaskRun would have references to Resources as well. That would certainly make it easier to create them by hand!

from pipeline.

my high level plan so far is the following:

• the git resource will return an array of init-containers,
  • the creds init similar to how the build project uses the service account
  • git resource init-container that will include the github repo fetched on a volume in a folder with the name specified as the name of the inputSourceBinding

I'm expecting the TaskRun Controller/Runner will take this array of init-containers and add them to the Pod that will actually execute the task

@bobcatfish @imjasonh @shashwathi @tejal29 makes sense?

from pipeline.

Sounds great to me @pivotal-nader-ziada ! I pinged @aaron-prindle in case he has any other comments, he's working on #59.

from pipeline.