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

Add support for Git submodules with go-git #327

Merged
merged 2 commits into from
Mar 31, 2021
Merged

Conversation

stefanprodan
Copy link
Member

@stefanprodan stefanprodan commented Mar 29, 2021

This PR adds an optional field .spec.recurseSubmodules to the GitRepository API. When enabled, after the clone is created, initializes all submodules within, using their default settings. This option is available only when using the go-git GitImplementation.

Fix: #169
Addresses: fluxcd/flux2#326

@stefanprodan stefanprodan added enhancement New feature or request area/git Git related issues and pull requests labels Mar 29, 2021
@stefanprodan stefanprodan force-pushed the fetch-submodules branch 2 times, most recently from 7386b7d to 19cf29e Compare March 29, 2021 17:22
@stefanprodan stefanprodan force-pushed the fetch-submodules branch 2 times, most recently from d3a9679 to 46343a8 Compare March 30, 2021 08:18
Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

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

API and implementation look 👍 I think some tests are warranted here though.

pkg/git/gogit/checkout.go Outdated Show resolved Hide resolved
@stefanprodan
Copy link
Member Author

stefanprodan commented Mar 30, 2021

I think some tests are warranted here though.

Wouldn't that be testing go-git cloning that's covered in go-git? Any hits how to write this test or how to make our Git server serve submodules?

For referece, I do test my PRs on real clusters:

Without submodules:

/ $ tar -tvf ./data/gitrepository/flux-system/sm-test/f51a795e39915fc3123aa7c8b7455b91e0203be8.tar.gz 
-rw-r--r-- controller/1337     11357 2021-03-30 08:37:37 LICENSE
-rw-r--r-- controller/1337      1645 2021-03-30 08:37:37 README.md
-rw-r--r-- controller/1337       509 2021-03-30 08:37:37 default/podinfo.yaml
-rw-r--r-- controller/1337       216 2021-03-30 08:37:37 default/service.yaml

With submodules:

/ $ tar -tvf ./data/gitrepository/flux-system/sm-test/f51a795e39915fc3123aa7c8b7455b91e0203be8.tar.gz 
-rw-r--r-- controller/1337     11357 2021-03-30 08:51:25 LICENSE
-rw-r--r-- controller/1337      1645 2021-03-30 08:51:25 README.md
-rw-r--r-- controller/1337       509 2021-03-30 08:51:25 default/podinfo.yaml
-rw-r--r-- controller/1337       216 2021-03-30 08:51:25 default/service.yaml
-rw-r--r-- controller/1337        32 2021-03-30 08:51:25 podinfo/.git
-rw-r--r-- controller/1337       892 2021-03-30 08:51:27 podinfo/.github/actions/helm/action.yml
-rw-r--r-- controller/1337       131 2021-03-30 08:51:27 podinfo/.github/actions/release-notes/Dockerfile
-rw-r--r-- controller/1337       211 2021-03-30 08:51:27 podinfo/.github/actions/release-notes/action.yml
-rw-r--r-- controller/1337       547 2021-03-30 08:51:27 podinfo/.github/actions/release-notes/entrypoint.sh
-rw-r--r-- controller/1337       770 2021-03-30 08:51:27 podinfo/.github/policy/kubernetes.rego
-rw-r--r-- controller/1337      1185 2021-03-30 08:51:27 podinfo/.github/policy/rules.rego
-rw-r--r-- controller/1337       587 2021-03-30 08:51:27 podinfo/.github/workflows/cve-scan.yml
-rw-r--r-- controller/1337       804 2021-03-30 08:51:27 podinfo/.github/workflows/e2e.yml
-rw-r--r-- controller/1337      3147 2021-03-30 08:51:27 podinfo/.github/workflows/release.yml
-rw-r--r-- controller/1337      1164 2021-03-30 08:51:27 podinfo/.github/workflows/test.yml
-rw-r--r-- controller/1337       356 2021-03-30 08:51:27 podinfo/.goreleaser.yml
-rw-r--r-- controller/1337       841 2021-03-30 08:51:27 podinfo/Dockerfile
-rw-r--r-- controller/1337       138 2021-03-30 08:51:27 podinfo/Dockerfile.base
-rw-r--r-- controller/1337     11365 2021-03-30 08:51:27 podinfo/LICENSE
-rw-r--r-- controller/1337      3341 2021-03-30 08:51:27 podinfo/Makefile
-rw-r--r-- controller/1337      7179 2021-03-30 08:51:27 podinfo/README.md
-rw-r--r-- controller/1337       333 2021-03-30 08:51:27 podinfo/charts/podinfo/.helmignore
-rw-r--r-- controller/1337       301 2021-03-30 08:51:27 podinfo/charts/podinfo/Chart.yaml
-rw-r--r-- controller/1337     11365 2021-03-30 08:51:27 podinfo/charts/podinfo/LICENSE
-rw-r--r-- controller/1337      5242 2021-03-30 08:51:27 podinfo/charts/podinfo/README.md
-rw-r--r-- controller/1337      1324 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/NOTES.txt
-rw-r--r-- controller/1337      1965 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/_helpers.tpl
-rw-r--r-- controller/1337       455 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/certificate.yaml
-rw-r--r-- controller/1337      5956 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/deployment.yaml
-rw-r--r-- controller/1337      1000 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/hpa.yaml
-rw-r--r-- controller/1337      1057 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/ingress.yaml
-rw-r--r-- controller/1337      2338 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/linkerd.yaml
-rw-r--r-- controller/1337       239 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/redis/config.yaml
-rw-r--r-- controller/1337      1928 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/redis/deployment.yaml
-rw-r--r-- controller/1337       369 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/redis/service.yaml
-rw-r--r-- controller/1337       988 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/service.yaml
-rw-r--r-- controller/1337       214 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/serviceaccount.yaml
-rw-r--r-- controller/1337       495 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/servicemonitor.yaml
-rw-r--r-- controller/1337       912 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/tests/cache.yaml
-rw-r--r-- controller/1337       607 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/tests/fail.yaml
-rw-r--r-- controller/1337       694 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/tests/grpc.yaml
-rw-r--r-- controller/1337       884 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/tests/jwt.yaml
-rw-r--r-- controller/1337       781 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/tests/service.yaml
-rw-r--r-- controller/1337       632 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/tests/timeout.yaml
-rw-r--r-- controller/1337       815 2021-03-30 08:51:27 podinfo/charts/podinfo/templates/tests/tls.yaml
-rw-r--r-- controller/1337      2746 2021-03-30 08:51:27 podinfo/charts/podinfo/values-prod.yaml
-rw-r--r-- controller/1337      2902 2021-03-30 08:51:27 podinfo/charts/podinfo/values.yaml
-rw-r--r-- controller/1337       212 2021-03-30 08:51:27 podinfo/cloudbuild.yaml
-rw-r--r-- controller/1337      7808 2021-03-30 08:51:27 podinfo/cmd/podcli/check.go
-rw-r--r-- controller/1337       574 2021-03-30 08:51:27 podinfo/cmd/podcli/main.go
-rw-r--r-- controller/1337       346 2021-03-30 08:51:27 podinfo/cmd/podcli/version.go
-rw-r--r-- controller/1337      2736 2021-03-30 08:51:27 podinfo/cmd/podcli/ws.go
-rw-r--r-- controller/1337      7624 2021-03-30 08:51:27 podinfo/cmd/podinfo/main.go
-rw-r--r-- controller/1337      1078 2021-03-30 08:51:27 podinfo/deploy/README.md
-rw-r--r-- controller/1337      1686 2021-03-30 08:51:27 podinfo/deploy/bases/backend/deployment.yaml
-rw-r--r-- controller/1337       340 2021-03-30 08:51:27 podinfo/deploy/bases/backend/hpa.yaml
-rw-r--r-- controller/1337       126 2021-03-30 08:51:27 podinfo/deploy/bases/backend/kustomization.yaml
-rw-r--r-- controller/1337       271 2021-03-30 08:51:27 podinfo/deploy/bases/backend/service.yaml
-rw-r--r-- controller/1337      1344 2021-03-30 08:51:27 podinfo/deploy/bases/cache/deployment.yaml
-rw-r--r-- controller/1337       185 2021-03-30 08:51:27 podinfo/deploy/bases/cache/kustomization.yaml
-rw-r--r-- controller/1337        66 2021-03-30 08:51:27 podinfo/deploy/bases/cache/redis.conf
-rw-r--r-- controller/1337       192 2021-03-30 08:51:27 podinfo/deploy/bases/cache/service.yaml
-rw-r--r-- controller/1337      1674 2021-03-30 08:51:27 podinfo/deploy/bases/frontend/deployment.yaml
-rw-r--r-- controller/1337       342 2021-03-30 08:51:27 podinfo/deploy/bases/frontend/hpa.yaml
-rw-r--r-- controller/1337       126 2021-03-30 08:51:27 podinfo/deploy/bases/frontend/kustomization.yaml
-rw-r--r-- controller/1337       194 2021-03-30 08:51:27 podinfo/deploy/bases/frontend/service.yaml
-rwxr-xr-x controller/1337      1307 2021-03-30 08:51:27 podinfo/deploy/kind.sh
-rw-r--r-- controller/1337       210 2021-03-30 08:51:27 podinfo/deploy/overlays/dev/kustomization.yaml
-rw-r--r-- controller/1337       161 2021-03-30 08:51:27 podinfo/deploy/overlays/dev/labels.yaml
-rw-r--r-- controller/1337        53 2021-03-30 08:51:27 podinfo/deploy/overlays/dev/namespace.yaml
-rw-r--r-- controller/1337       217 2021-03-30 08:51:27 podinfo/deploy/overlays/production/kustomization.yaml
-rw-r--r-- controller/1337       168 2021-03-30 08:51:27 podinfo/deploy/overlays/production/labels.yaml
-rw-r--r-- controller/1337        60 2021-03-30 08:51:27 podinfo/deploy/overlays/production/namespace.yaml
-rw-r--r-- controller/1337       214 2021-03-30 08:51:27 podinfo/deploy/overlays/staging/kustomization.yaml
-rw-r--r-- controller/1337       165 2021-03-30 08:51:27 podinfo/deploy/overlays/staging/labels.yaml
-rw-r--r-- controller/1337        57 2021-03-30 08:51:27 podinfo/deploy/overlays/staging/namespace.yaml
-rw-r--r-- controller/1337      1703 2021-03-30 08:51:27 podinfo/deploy/secure/backend/deployment.yaml
-rw-r--r-- controller/1337       360 2021-03-30 08:51:27 podinfo/deploy/secure/backend/hpa.yaml
-rw-r--r-- controller/1337       291 2021-03-30 08:51:27 podinfo/deploy/secure/backend/service.yaml
-rw-r--r-- controller/1337       103 2021-03-30 08:51:27 podinfo/deploy/secure/common/cluster-issuer.yaml
-rw-r--r-- controller/1337        56 2021-03-30 08:51:27 podinfo/deploy/secure/common/namespace.yaml
-rw-r--r-- controller/1337       525 2021-03-30 08:51:27 podinfo/deploy/secure/common/reconciler-rbac.yaml
-rw-r--r-- controller/1337        81 2021-03-30 08:51:27 podinfo/deploy/secure/common/service-account.yaml
-rw-r--r-- controller/1337       305 2021-03-30 08:51:27 podinfo/deploy/secure/frontend/certificate.yaml
-rw-r--r-- controller/1337      2237 2021-03-30 08:51:27 podinfo/deploy/secure/frontend/deployment.yaml
-rw-r--r-- controller/1337       362 2021-03-30 08:51:27 podinfo/deploy/secure/frontend/hpa.yaml
-rw-r--r-- controller/1337       291 2021-03-30 08:51:27 podinfo/deploy/secure/frontend/service.yaml
-rw-r--r-- controller/1337      1703 2021-03-30 08:51:27 podinfo/deploy/webapp/backend/deployment.yaml
-rw-r--r-- controller/1337       360 2021-03-30 08:51:27 podinfo/deploy/webapp/backend/hpa.yaml
-rw-r--r-- controller/1337       291 2021-03-30 08:51:27 podinfo/deploy/webapp/backend/service.yaml
-rw-r--r-- controller/1337        56 2021-03-30 08:51:27 podinfo/deploy/webapp/common/namespace.yaml
-rw-r--r-- controller/1337       525 2021-03-30 08:51:27 podinfo/deploy/webapp/common/reconciler-rbac.yaml
-rw-r--r-- controller/1337        81 2021-03-30 08:51:27 podinfo/deploy/webapp/common/service-account.yaml
-rw-r--r-- controller/1337      1691 2021-03-30 08:51:27 podinfo/deploy/webapp/frontend/deployment.yaml
-rw-r--r-- controller/1337       361 2021-03-30 08:51:27 podinfo/deploy/webapp/frontend/hpa.yaml
-rw-r--r-- controller/1337       214 2021-03-30 08:51:27 podinfo/deploy/webapp/frontend/service.yaml
-rw-r--r-- controller/1337       892 2021-03-30 08:51:27 podinfo/go.mod
-rw-r--r-- controller/1337     64430 2021-03-30 08:51:27 podinfo/go.sum
-rw-r--r-- controller/1337      1713 2021-03-30 08:51:27 podinfo/kustomize/deployment.yaml
-rw-r--r-- controller/1337       419 2021-03-30 08:51:27 podinfo/kustomize/hpa.yaml
-rw-r--r-- controller/1337       126 2021-03-30 08:51:27 podinfo/kustomize/kustomization.yaml
-rw-r--r-- controller/1337       271 2021-03-30 08:51:27 podinfo/kustomize/service.yaml
-rw-r--r-- controller/1337      3477 2021-03-30 08:51:27 podinfo/pkg/api/cache.go
-rw-r--r-- controller/1337      1054 2021-03-30 08:51:27 podinfo/pkg/api/chunked.go
-rw-r--r-- controller/1337       803 2021-03-30 08:51:27 podinfo/pkg/api/chunked_test.go
-rw-r--r-- controller/1337       329 2021-03-30 08:51:27 podinfo/pkg/api/configs.go
-rw-r--r-- controller/1337      1342 2021-03-30 08:51:27 podinfo/pkg/api/delay.go
-rw-r--r-- controller/1337       795 2021-03-30 08:51:27 podinfo/pkg/api/delay_test.go
-rw-r--r-- controller/1337     19419 2021-03-30 08:51:27 podinfo/pkg/api/docs/docs.go
-rw-r--r-- controller/1337     18254 2021-03-30 08:51:27 podinfo/pkg/api/docs/swagger.json
-rw-r--r-- controller/1337      8896 2021-03-30 08:51:27 podinfo/pkg/api/docs/swagger.yaml
-rw-r--r-- controller/1337      2763 2021-03-30 08:51:27 podinfo/pkg/api/echo.go
-rw-r--r-- controller/1337       781 2021-03-30 08:51:27 podinfo/pkg/api/echo_test.go
-rw-r--r-- controller/1337      1883 2021-03-30 08:51:27 podinfo/pkg/api/echows.go
-rw-r--r-- controller/1337       371 2021-03-30 08:51:27 podinfo/pkg/api/env.go
-rw-r--r-- controller/1337       778 2021-03-30 08:51:27 podinfo/pkg/api/env_test.go
-rw-r--r-- controller/1337       373 2021-03-30 08:51:27 podinfo/pkg/api/headers.go
-rw-r--r-- controller/1337       859 2021-03-30 08:51:27 podinfo/pkg/api/headers_test.go
-rw-r--r-- controller/1337      1751 2021-03-30 08:51:27 podinfo/pkg/api/health.go
-rw-r--r-- controller/1337      1035 2021-03-30 08:51:27 podinfo/pkg/api/health_test.go
-rw-r--r-- controller/1337      2852 2021-03-30 08:51:27 podinfo/pkg/api/http.go
-rw-r--r-- controller/1337       795 2021-03-30 08:51:27 podinfo/pkg/api/index.go
-rw-r--r-- controller/1337      1342 2021-03-30 08:51:27 podinfo/pkg/api/info.go
-rw-r--r-- controller/1337       783 2021-03-30 08:51:27 podinfo/pkg/api/info_test.go
-rw-r--r-- controller/1337       633 2021-03-30 08:51:27 podinfo/pkg/api/logging.go
-rw-r--r-- controller/1337     10170 2021-03-30 08:51:27 podinfo/pkg/api/metrics.go
-rw-r--r-- controller/1337       708 2021-03-30 08:51:27 podinfo/pkg/api/mock.go
-rw-r--r-- controller/1337       283 2021-03-30 08:51:27 podinfo/pkg/api/panic.go
-rw-r--r-- controller/1337     10735 2021-03-30 08:51:27 podinfo/pkg/api/server.go
-rw-r--r-- controller/1337       600 2021-03-30 08:51:27 podinfo/pkg/api/status.go
-rw-r--r-- controller/1337       549 2021-03-30 08:51:27 podinfo/pkg/api/status_test.go
-rw-r--r-- controller/1337      1830 2021-03-30 08:51:27 podinfo/pkg/api/store.go
-rw-r--r-- controller/1337      3258 2021-03-30 08:51:27 podinfo/pkg/api/token.go
-rw-r--r-- controller/1337       484 2021-03-30 08:51:27 podinfo/pkg/api/version.go
-rw-r--r-- controller/1337       811 2021-03-30 08:51:27 podinfo/pkg/api/version_test.go
-rw-r--r-- controller/1337      2244 2021-03-30 08:51:27 podinfo/pkg/fscache/fscache.go
-rw-r--r-- controller/1337      1059 2021-03-30 08:51:27 podinfo/pkg/grpc/server.go
-rw-r--r-- controller/1337       615 2021-03-30 08:51:27 podinfo/pkg/signals/signal.go
-rw-r--r-- controller/1337       131 2021-03-30 08:51:27 podinfo/pkg/signals/signal_posix.go
-rw-r--r-- controller/1337        83 2021-03-30 08:51:27 podinfo/pkg/signals/signal_windows.go
-rw-r--r-- controller/1337        64 2021-03-30 08:51:27 podinfo/pkg/version/version.go
-rwxr-xr-x controller/1337       181 2021-03-30 08:51:27 podinfo/test/build.sh
-rwxr-xr-x controller/1337       888 2021-03-30 08:51:27 podinfo/test/deploy.sh
-rwxr-xr-x controller/1337       325 2021-03-30 08:51:27 podinfo/test/e2e.sh
-rwxr-xr-x controller/1337       136 2021-03-30 08:51:27 podinfo/test/test.sh
-rw-r--r-- controller/1337      5456 2021-03-30 08:51:27 podinfo/ui/vue.html

@squaremo
Copy link
Member

I think some tests are warranted here though.

Wouldn't that be testing go-git cloning that's covered in go-git?

[and]

For reference, I do test my PRs on real clusters:

I believe you. But did you test in advance all future changes to the code to make sure they didn't e.g., accidentally invert, or omit, the argument that is given to go-git? ;-)

Any hits how to write this test or how to make our Git server serve submodules?

I don't think you need to do anything special to the git server for submodules to work, other than serve more than one repo -- they are a client-side feature. There's already a bunch of scaffolding for creating a git server and getting stuff from it in gitrepository_controller_test.go, this won't be much of a stretch beyond that will it?

@stefanprodan
Copy link
Member Author

There's already a bunch of scaffolding for creating a git server and getting stuff from it in gitrepository_controller_test.go, this won't be much of a stretch beyond that will it?

I see no files being pushed to Git, can you give some hints how to setup submodules, I'm lost.

Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
@squaremo
Copy link
Member

I see no files being pushed to Git, can you give some hints how to setup submodules, I'm lost.

These lines init a git repo, create a file, commit it, and push to the test server: https://github.com/fluxcd/source-controller/blob/main/controllers/gitrepository_controller_test.go#L120-L156

If you prefer getting the files from testdata/, here's the equivalent from image-automation, that walks the filesystem and adds files from there: https://github.com/fluxcd/image-automation-controller/blob/main/controllers/update_test.go#L860

As for submodules: I haven't created submodules using go-git. It looks like go-git understands submodules (https://github.com/go-git/go-git/blob/master/_examples/submodule/main.go), but I'm not sure if it will create them. Might take a bit of fishing around in the go-git docs.

@squaremo squaremo force-pushed the fetch-submodules branch 2 times, most recently from 225ecf5 to 2ef4c22 Compare March 31, 2021 08:41
This commit adds a test specifically for RecurseSubmodules. It takes a
bit more preparation, since it needs a repo using submodules to start
with. go-git doesn't appear to support adding submodules
programmatically, so the preparation is done in part by execing `git`.

Signed-off-by: Michael Bridgen <michael@weave.works>
@stefanprodan stefanprodan merged commit 00ca53c into main Mar 31, 2021
@stefanprodan stefanprodan deleted the fetch-submodules branch March 31, 2021 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Git submodules
3 participants