Remove scaffolding images

[bouskaJ]

Fixes #3 .

As wrote in the README.md file. This PR disable locally running operator because the operator connect directly to the trillian component. The communication is done through RPC. The RPC protocol does rely on HTTP/2 and this traffic is hard (but not impossible - see https://www.redhat.com/en/blog/grpc-or-http/2-ingress-connectivity-in-openshift) to route outside the cluster.

I will open new issue for fixing that.

[cooktheryan]

@bouskaJ how does the helm charts handle this today? Also, if we have the svc address defined in the configuration do we have the capabilities of bypassing the OpenShift Haproxy?

[bouskaJ]

@bouskaJ how does the helm charts handle this today?

Helm does not have any active member so there is no capability to run outside the cluster. It creates Job resources that runs always in the cluster. Also handling the configuration is quite hackish and it was solved by Jobs that are based on images from the https://github.com/securesign/scaffolding project and it brings additional cost of productization and maintenance. Also, it is hard to synchronize them so you can always see a lot of failed pods, restarting and waiting for something.

Also, if we have the svc address defined in the configuration do we have the capabilities of bypassing the OpenShift Haproxy?

I am not sure what you mean by this. The svc addresses are routed inside the cluster by default. Routing them outside is done by Route resource (that works only for http traffic and can be enhanced for the RPC) or using the nodeport.

I think both solutions (RPC route or nodeport) are possible, it just need some time to dig into it. The benefit of removing the dependency on the scaffolding images is worth that IMHO.

[bouskaJ]

@lance do you want to comment?

[bouskaJ]

I managed to workaround the issue using https://www.redhat.com/en/blog/grpc-or-http/2-ingress-connectivity-in-openshift. I will make it part of this PR.

[cooktheryan]

@sallyom or i can look at the code but @lance going to assign you as the main reviewer based on the above.

cc @lkatalin

[cooktheryan]

@sallyom or i can look at the code but @lance going to assign you as the main reviewer based on the above.

cc @lkatalin

[lance]

Also handling the configuration is quite hackish and it was solved by Jobs that are based on images from the https://github.com/securesign/scaffolding project and it brings additional cost of productization and maintenance. Also, it is hard to synchronize them so you can always see a lot of failed pods, restarting and waiting for something.

I agree with this. I have never really liked all of the scaffolding images we are required to produce for helm installation. These do things like initialize the database and initial service configuration. With the exception of backfill-redis none of these images are used after a Helm install.

The fact that these same simple processes can be easily incorporated into a Go operator is nice and yes, it does significantly reduce our internal resource utilization for builds, and makes product releases simpler, as we will eliminate around a half dozen images that we need to ship.

[lance]

Looking good! I left a few comments inline. Also, I like that we are using the app.kubernetes.io.part-of label for the application name. 👍

It looks like the trillian bits still need work on the initialization phases. I could be missing something, but I did not see where the database tables are created, for example.

I did not check every line of the ct_log initialization and compare with https://github.com/securesign/scaffolding/blob/redhat-v0.6/cmd/ctlog/createctconfig/main.go either. Is that scaffolding image the basis for the init code in, for example controllers/ctlog/create.go?

Overall, I like the pattern.

I think it's important that we get all of the initialization steps done correctly, of course. It might be nice to reference the scaffolding code in comments here, so that it's clear where the behavior is derived from. This also introduces a risk that scaffolding and the operator could diverge - something we need to be on top of.

This does seem to set us back a little bit with regard to BYO certs/keys/db/etc. doesn't it?

[lance]

I wonder if utilities like this should always inject a part-of label. Or maybe that's what's always passed in... Just wondering if having the part-of labels always added at the point of object creation would help with consistency. Take it or leave it. :)

[bouskaJ]

It's always passed in the labels map that is parsed by https://github.com/securesign/secure-sign-operator/blob/labels/controllers/common/utils/kubernetes/common.go#L6 but maybe it make sense to have it explicitly defined. 👍 WIll fix it.

[lance]

The same comment here as from CreateConfigMap regarding labels.

[lance]

How will we handle customers who want to bring their own certificates?

[bouskaJ]

There is possibility to set the secretName in the CRD (https://github.com/securesign/secure-sign-operator/blob/main/api/v1alpha1/fulcio_types.go#L13) the automatic key generation is optional and it was added in #14 (we will need to tidy the API once it's fully defined to be more understandable)

[bouskaJ]

@lance I think we are good to go. I would left the custom DB support (and the related create-db job), API refactoring and better documentation for follow-up PRs.

[lance]

/lgtm