Implement git repository controller #82

nabuskey · 2023-11-14T22:52:08Z

This implements the git repository types and controller.

Syncs the contents of a local directory to an in-cluster gitea repository for ArgoCD to pick up.

We need #39 merged first.

closes #59, closes #60

nabuskey · 2023-11-16T23:29:42Z

pkg/controllers/gitrepository/controller.go

+
+const (
+	DefaultBranchName = "main"
+	// DefaultGiteaUsername and DefaultGiteaPassword taken from gitea helm chart. https://gitea.com/gitea/helm-chart#gitea


We need a way to get these values dynamically. If the gitea object can expose that information, we can do so. Otherwise we will have to rely on a secret naming convention. e.g. we will always look for a secret named abc in namespace abc.

nimakaviani

Looks neat, thanks. left a few comments.

Also, looks like the bit that actually hooks things up and runs this in RunControllers is not here in this commit. intentional?

pkg/controllers/gitrepository/controller.go

api/v1alpha1/gitrepository_types.go

pkg/controllers/gitrepository/controller.go

nimakaviani · 2023-11-17T01:57:51Z

also looks like you've got some conflicts to resolve

pkg/controllers/gitrepository/controller.go

api/v1alpha1/gitrepository_types.go

nabuskey · 2023-11-21T01:43:10Z

This PR's become a bit too large for my liking but it implements the entire workflow with Gitea.

All emebdded manifests are pushed to gitea server under the admin user organization. We may want to make this configurable in the future.

Once they are pushed, argo applications are created. They reference the repos created above. From here, ArgoCD takes care of the rest. The Git repository contents are reconciled only once for embedded applications. If users wants to reconfigure things, they can do so in the Gitea UI.

Once this PR merges, I just need to rebase and build should pass.

push manifests to gitea Signed-off-by: Manabu Mccloskey <[email protected]>

Signed-off-by: Manabu Mccloskey <[email protected]>

nimakaviani

few minor updates. otherwise looks good.

One higher level question, for the backstage embeded app, shall we use the helm chart with its image set to our CNOE backstage image and let Argo CD handle it, rather than having the raw resources?

api/v1alpha1/localbuild_types.go

pkg/controllers/gitrepository/controller.go

pkg/controllers/gitrepository/resources/file1

nimakaviani · 2023-11-22T20:53:55Z

pkg/controllers/localbuild/controller.go

-	log := log.FromContext(ctx)
+	logger := log.FromContext(ctx)


i think we have called the variable log in other places. would be good to make this consistent across the board and btw i like logger better.

Problem with using log is that it collides with package import like this

idpbuilder/pkg/controllers/localbuild/controller.go

Line 21 in 09608f0

"sigs.k8s.io/controller-runtime/pkg/log"

I agree that we should use logger instead of log across the project. I'll file an issue.

nimakaviani · 2023-11-22T21:00:19Z

pkg/controllers/localbuild/gitea.go

 	giteaAdminSecret = "gitea-admin-secret"
+	// this is the URL accessible outside cluster. resolves to localhost
+	giteaIngressURL = "http://gitea.cnoe.localtest.me:8880"


8443 or 8880 for the port number?

Should be 8880. We should consider using TLS in the future, but it's all http for gitea as of now.

Should be 8880. We should consider using TLS in the future, but it's all http for gitea as of now.

I don t think that we should append additional ports to the URL as we do now:

https://argocd.<host_IP>.<DOMAIN>:8443,

https://gitea.<host_IP>.<DOMAIN>:8880,

https://backstage.<host_IP>.<DOMAIN>:8443,

etc

Why: ?

Adding more and more new ports will make the work of the users not really cool and easy as they will have to remember the ports to be used for each component

Only default HTTP(S) ports are needed and will be recognized by the browser: 80, 443 automatically.

As the ingress controller which is acting as a reverse proxy using nginx is able to forward from URL: https://backend>.<host_ip>.<domain> the packets to the target backends: argocd, backstage, vault, gitea, etc then adding new ports make the job to configure ingress + kind part of the idpbuilder hardder instead of having Ingress YAML files

I agree from the user friendliness standpoint. The reason we are not using default http and https ports is that allocation of lower number port (< 1024) requires higher privileges that some organizations do not allow end users to have. I would like to use default ports but I would rather have the assurance that port allocation will work for all users. Maybe we can make it configurable in the future.

cmoulliard · 2023-11-23T10:44:13Z

api/v1alpha1/gitrepository_types.go

+	// Type is the source type.
+	// +kubebuilder:validation:Enum:=local;embedded
+	// +kubebuilder:default:=embedded
+	Type string `json:"type"`


Can you describe what is the purpose of the type and what will happen if we select one the values: local, embedded, etc ?

Local indicates the gitea repository contents will come from a local directory. E.g. /home/ubuntu/myrepo

Embedded indicates the repository contents will come from what is embedded in the go binary.

cmoulliard · 2023-11-23T10:44:56Z

api/v1alpha1/gitrepository_types.go

+}
+
+type GitRepositorySource struct {
+	// +kubebuilder:validation:Enum:=argocd;backstage;crossplane;gitea;nginx


crossplane should be removed from the enum and we should keep what currently we support or point to a link = doc page enumerating them

We will probably have to keep it since the reference implementation uses it. For now, I am keeping it because the purpose of this PR is to move the git functionality over from embedded git server to gitea.

cmoulliard · 2023-11-23T10:46:58Z

api/v1alpha1/gitrepository_types.go

+	ExternalGitRepositoryUrl string `json:"externalGitRepositoryUrl"`
+	// GitRepositoryUrl is the url for the in-cluster repository accessible within the cluster.
+	// +kubebuilder:validation:Optional
+	GitRepositoryUrl string `json:"gitRepositoryUrl"`


Why don't you name it InternalGitRepositoryUrl as it is used for internal purposes ? If we use gitea which is accessible by the users, does that makes sense that we keep ext vs internal repos ?

Good idea I will rename the field.

We need to distinct the internal and external git repo URLs (gitea.cnoe.localtest.me) because the domain name will always resolve to 127.0.0.1. Therefore this URL is not usable by ArgoCD to fetch manifests from the gitea server.

Signed-off-by: Manabu Mccloskey <[email protected]>

nabuskey marked this pull request as draft November 14, 2023 22:53

nabuskey changed the title ~~Feature/git controller~~ Implement git repository controller Nov 14, 2023

nabuskey force-pushed the feature/git-controller branch 2 times, most recently from cd01ed4 to 6538ae2 Compare November 16, 2023 23:26

nabuskey marked this pull request as ready for review November 16, 2023 23:26

nabuskey commented Nov 16, 2023

View reviewed changes

nimakaviani requested review from greghaynes and jessesanford November 17, 2023 01:20

nimakaviani reviewed Nov 17, 2023

View reviewed changes

pkg/controllers/gitrepository/controller.go Outdated Show resolved Hide resolved

greghaynes reviewed Nov 17, 2023

View reviewed changes

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

greghaynes reviewed Nov 17, 2023

View reviewed changes

api/v1alpha1/gitrepository_types.go Show resolved Hide resolved

nabuskey force-pushed the feature/git-controller branch 2 times, most recently from dd48340 to cb6156f Compare November 21, 2023 01:34

add initial support for local and embdedded fs

a2a8208

push manifests to gitea Signed-off-by: Manabu Mccloskey <[email protected]>

nabuskey force-pushed the feature/git-controller branch from cb6156f to d27a5ae Compare November 21, 2023 19:18

implement shutdown

6dd8cda

Signed-off-by: Manabu Mccloskey <[email protected]>

nabuskey force-pushed the feature/git-controller branch from d27a5ae to 6dd8cda Compare November 21, 2023 19:22

remove unused files

09608f0

Signed-off-by: Manabu Mccloskey <[email protected]>

nimakaviani approved these changes Nov 22, 2023

View reviewed changes

cmoulliard reviewed Nov 23, 2023

View reviewed changes

address review comments

9d00824

Signed-off-by: Manabu Mccloskey <[email protected]>

nabuskey force-pushed the feature/git-controller branch from d7ae0fe to 9d00824 Compare November 24, 2023 17:36

nabuskey merged commit ca8f132 into cnoe-io:main Nov 24, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement git repository controller #82

Implement git repository controller #82

nabuskey commented Nov 14, 2023

nabuskey Nov 16, 2023 •

edited

Loading

nimakaviani left a comment

nimakaviani commented Nov 17, 2023

nabuskey commented Nov 21, 2023 •

edited

Loading

nimakaviani left a comment

nimakaviani Nov 22, 2023

nabuskey Nov 22, 2023

nimakaviani Nov 22, 2023

nabuskey Nov 22, 2023

cmoulliard Nov 23, 2023

nabuskey Nov 24, 2023

cmoulliard Nov 23, 2023

nabuskey Nov 24, 2023

cmoulliard Nov 23, 2023

nabuskey Nov 24, 2023

cmoulliard Nov 23, 2023

nabuskey Nov 24, 2023 •

edited

Loading

Implement git repository controller #82

Implement git repository controller #82

Conversation

nabuskey commented Nov 14, 2023

nabuskey Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

nimakaviani left a comment

Choose a reason for hiding this comment

nimakaviani commented Nov 17, 2023

nabuskey commented Nov 21, 2023 • edited Loading

nimakaviani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nabuskey Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

nabuskey Nov 16, 2023 •

edited

Loading

nabuskey commented Nov 21, 2023 •

edited

Loading

nabuskey Nov 24, 2023 •

edited

Loading