Add include property to GitRepositories #348

phillebaba · 2021-04-22T10:32:41Z

This change will introduce a new field called include which will allow users to map a GitRepository into another GitRepository. The feature has been discussed in fluxcd/flux2#326 (comment) and is based on Stefans suggestion. The idea is that one GitRepository should be able to include the contents of another GitRepository, similar to how git submodules works.

This is meant to be an alternative to git submodules, here are some of the differences between the solutions:

Including a GitRepository allows you to use different authentication methods for different repositories.
A change in the included repository will trigger an update of the including repository.
Multiple GitRepositories could include the same repository, which decreases the amount of cloning done compared to using submodules.

Example

I have setup an example repository which uses this new feature, please refer to it for an example if you want to try it out. It shows a simple example by creating a GitRepository source that also uses the fleet-infra repository but includes the content of the podinfo repository in it, so that it can use its manifest without using kustomize remote refs.
https://github.com/phillebaba/fleet-infra

The apps GitRepository in the source which includes the podinfo GitRepository. I you look at spec.include you can see that it references another GitRepository. This GitRepository now contains the contents of the podinfo repository in it under the path apps/podinfo/sub.

apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: GitRepository
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"source.toolkit.fluxcd.io/v1beta1","kind":"GitRepository","metadata":{"annotations":{},"labels":{"kustomize.toolkit.fluxcd.io/checksum":"6e219ac79633fb30ebed823019c595b46cd3df32","kustomize.toolkit.fluxcd.io/name":"flux-system","kustomize.toolkit.fluxcd.io/namespace":"flux-system"},"name":"apps","namespace":"flux-system"},"spec":{"include":[{"path":"apps/podinfo/sub","repository":{"name":"podinfo","namespace":"flux-system"}}],"interval":"30s","ref":{"branch":"main"},"url":"https://github.com/phillebaba/fleet-infra.git"}}
    reconcile.fluxcd.io/requestedAt: "2021-04-26T16:09:14.342492956+02:00"
  creationTimestamp: "2021-04-26T13:58:36Z"
  finalizers:
  - finalizers.fluxcd.io
  generation: 7
  labels:
    kustomize.toolkit.fluxcd.io/checksum: 6e219ac79633fb30ebed823019c595b46cd3df32
    kustomize.toolkit.fluxcd.io/name: flux-system
    kustomize.toolkit.fluxcd.io/namespace: flux-system
  name: apps
  namespace: flux-system
  resourceVersion: "19677"
  uid: 33cec1c5-8da1-4b19-898b-23f141d5da94
spec:
  gitImplementation: go-git
  include:
  - path: apps/podinfo/sub
    repository:
      name: podinfo
      namespace: flux-system
  interval: 30s
  ref:
    branch: main
  timeout: 20s
  url: https://github.com/phillebaba/fleet-infra.git
status:
  artifact:
    checksum: 6d7b1cc0cb4a5d1f8d7b64e8be64994ec76caadd
    lastUpdateTime: "2021-04-26T15:10:19Z"
    path: gitrepository/flux-system/apps/d1e2aae0646e39ffb2263d87932b71d64ff6ecd7.tar.gz
    revision: main/d1e2aae0646e39ffb2263d87932b71d64ff6ecd7
    url: http://source-controller.flux-system.svc.cluster.local./gitrepository/flux-system/apps/d1e2aae0646e39ffb2263d87932b71d64ff6ecd7.tar.gz
  conditions:
  - lastTransitionTime: "2021-04-26T14:09:20Z"
    message: 'Fetched revision: main/d1e2aae0646e39ffb2263d87932b71d64ff6ecd7'
    reason: GitOperationSucceed
    status: "True"
    type: Ready
  includeArtifcats:
  - checksum: 17ea18ddb232c59d648e0bace17c43fa55edbf00
    lastUpdateTime: "2021-04-26T14:04:59Z"
    path: gitrepository/flux-system/podinfo/21c8dfbb69bbd39e6838858159a2eac5577710d1.tar.gz
    revision: master/21c8dfbb69bbd39e6838858159a2eac5577710d1
    url: http://source-controller.flux-system.svc.cluster.local./gitrepository/flux-system/podinfo/21c8dfbb69bbd39e6838858159a2eac5577710d1.tar.gz
  lastHandledReconcileAt: "2021-04-26T16:09:14.342492956+02:00"
  observedGeneration: 7
  url: http://source-controller.flux-system.svc.cluster.local./gitrepository/flux-system/apps/latest.tar.gz

Open Questions

In a multi-tenant solution the tenants GitRepository will be created from the fleet-infra repository. This creates a weird situation where the you will need to create two GitRepository sources just so that you can do the include.
Should more source kinds other than GitRepository be supported? While possible it will introduce other challenges when determining if a included source has changed.

api/v1beta1/gitrepository_types.go

controllers/gitrepository_controller.go

phillebaba · 2021-04-22T10:35:43Z

controllers/gitrepository_controller.go

+
+// hasArtifactUpdated returns true if any of the revisions in the current artifacts
+// does not match any of the artifacts in the updated artifacts
+func hasArtifactUpdated(current []*sourcev1.Artifact, updated []*sourcev1.Artifact) bool {


There is probably a more efficient method of implementing this.

Also wondering if this has a zero chance of giving false negatives, as on a quick glance it looks like that would be possible if updated contains two versions that exist in current but for artifacts from different sources.

Would that not require two sources to have colliding hashes? I will add a url check to reduce the risk if this ever happens.

Ok so I actually missed a small thing which could have messed things up. I added tests which verify the logic, hopefully this lays any worries to rest @hiddeco.
https://github.com/fluxcd/source-controller/blob/c418c1c0eca890c0030d99cd6eff99c9658a0823/controllers/artifact_test.go

stefanprodan

Can you please fix this typo Artifcats to Artifacts, it's in the API spec and also in func names.

controllers/gitrepository_controller.go

controllers/gitrepository_controller_test.go

controllers/storage.go

controllers/gitrepository_controller.go

api/v1beta1/gitrepository_types.go

relu · 2021-04-22T16:08:28Z

I realize now that technically any of the source controller resources could be composed like this. Is there any particular reason why this would only be available for GitRepository?

phillebaba · 2021-04-26T14:21:24Z

@relu yes you are right that this could in theory work for any resource type inside another. The reason I started with GitRepositories is because it would be the easiest one. Other resource types like buckets do not have the concept of a revision so determining change may be a bit more difficult. To future proof this solution including a kind property in the resource ref may be smart.

api/v1beta1/gitrepository_types.go

phillebaba · 2021-05-05T13:35:22Z

I think I have addressed the majority of comments now. I just cant figure out why the docs gen is creating a different output compared to me running locally. Would appreciate it if anyone sees the obvious solution.

simongottschlag · 2021-05-05T16:42:23Z

I think I have addressed the majority of comments now. I just cant figure out why the docs gen is creating a different output compared to me running locally. Would appreciate it if anyone sees the obvious solution.

suggestions:

run the tests from the container: https://github.com/fluxcd/source-controller/blob/main/.github/actions/run-tests/Dockerfile
do a clean clone/checkout of the branch

stefanprodan · 2021-05-06T08:15:09Z

@phillebaba can you please bump the docs gen to github.com/ahmetb/gen-crd-api-reference-docs@v0.3.0

phillebaba · 2021-05-10T12:15:31Z

@stefanprodan so the issue seemed to have something to do with running the docs gen inside of go/src/github.com/fluxcd/source-controller. Moving the project out of their solved the issue for now. We should be all set now as long as there are no other changes required.

relu

Looks good, nice work @phillebaba! 💯

stefanprodan · 2021-05-11T07:30:15Z

@phillebaba please don't merge with upstream but instead rebase.

Signed-off-by: Philip Laine <philip.laine@gmail.com> Signed-off-by: Philip Laine <philip.laine@xenit.se>

stefanprodan

LGTM

Thanks @phillebaba 🥇

hiddeco · 2021-05-11T14:05:13Z

@phillebaba this is non-blocking for merge, but can we please document the "this feature is not supposed to grow" which was discussed during the community meeting awhile back?

phillebaba · 2021-05-11T15:22:38Z

@hiddeco I will merge and create a new PR that expands on this topic in the docs.

phillebaba marked this pull request as draft April 22, 2021 10:32

phillebaba commented Apr 22, 2021

View reviewed changes

phillebaba force-pushed the feature/include-source branch from 2eb3d67 to 50d7109 Compare April 22, 2021 10:37

stefanprodan reviewed Apr 22, 2021

View reviewed changes

relu requested changes Apr 22, 2021

View reviewed changes

phillebaba force-pushed the feature/include-source branch from 50d7109 to 9458014 Compare April 26, 2021 14:22

phillebaba force-pushed the feature/include-source branch from 9458014 to fb1084b Compare May 5, 2021 09:14

phillebaba marked this pull request as ready for review May 5, 2021 09:14

stefanprodan reviewed May 5, 2021

View reviewed changes

api/v1beta1/gitrepository_types.go Outdated Show resolved Hide resolved

phillebaba force-pushed the feature/include-source branch 5 times, most recently from 4832b2b to 0c13ff8 Compare May 5, 2021 13:33

phillebaba requested a review from stefanprodan May 6, 2021 05:48

phillebaba force-pushed the feature/include-source branch 4 times, most recently from 8575c45 to 82e5a9a Compare May 10, 2021 11:54

stefanprodan requested a review from relu May 10, 2021 12:46

relu approved these changes May 10, 2021

View reviewed changes

Add include property to GitRepositories

fcf7048

Signed-off-by: Philip Laine <philip.laine@gmail.com> Signed-off-by: Philip Laine <philip.laine@xenit.se>

phillebaba force-pushed the feature/include-source branch from 7144106 to fcf7048 Compare May 11, 2021 07:47

stefanprodan approved these changes May 11, 2021

View reviewed changes

stefanprodan added area/git Git related issues and pull requests enhancement New feature or request labels May 11, 2021

phillebaba merged commit 2dc7132 into main May 11, 2021

phillebaba deleted the feature/include-source branch May 11, 2021 15:23

jeffwestaway mentioned this pull request Jun 8, 2021

Cannot create GitRepository with Include feature fluxcd/terraform-provider-flux#163

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add include property to GitRepositories #348

Add include property to GitRepositories #348

phillebaba commented Apr 22, 2021 •

edited

Loading

phillebaba Apr 22, 2021

hiddeco Apr 26, 2021

phillebaba May 5, 2021

phillebaba May 5, 2021

stefanprodan left a comment

relu commented Apr 22, 2021

phillebaba commented Apr 26, 2021

phillebaba commented May 5, 2021

simongottschlag commented May 5, 2021

stefanprodan commented May 6, 2021

phillebaba commented May 10, 2021

relu left a comment

stefanprodan commented May 11, 2021

stefanprodan left a comment

hiddeco commented May 11, 2021

phillebaba commented May 11, 2021

Add include property to GitRepositories #348

Add include property to GitRepositories #348

Conversation

phillebaba commented Apr 22, 2021 • edited Loading

Example

Open Questions

phillebaba Apr 22, 2021

Choose a reason for hiding this comment

hiddeco Apr 26, 2021

Choose a reason for hiding this comment

phillebaba May 5, 2021

Choose a reason for hiding this comment

phillebaba May 5, 2021

Choose a reason for hiding this comment

stefanprodan left a comment

Choose a reason for hiding this comment

relu commented Apr 22, 2021

phillebaba commented Apr 26, 2021

phillebaba commented May 5, 2021

simongottschlag commented May 5, 2021

stefanprodan commented May 6, 2021

phillebaba commented May 10, 2021

relu left a comment

Choose a reason for hiding this comment

stefanprodan commented May 11, 2021

stefanprodan left a comment

Choose a reason for hiding this comment

hiddeco commented May 11, 2021

phillebaba commented May 11, 2021

phillebaba commented Apr 22, 2021 •

edited

Loading