Skip to content

Commit

Permalink
Use Certificates instead of CertificateRequests
Browse files Browse the repository at this point in the history
As discussed in
#49 ,
using `CertificateRequest` resources for interacting with cert-manager
has several drawbacks:

- this project (openshift-routes) has to duplicate logic for handling
the creation of certificates that already exists in cert-manager.
- observability is impaired because cert-manager does not offer
metrics for `CertificateRequest` resources, only for `Certificates`.

Therefore, this patch replaces the management of `CertificateRequests` with
`Certificates` (and `Secrets`). In return, it requires slightly higher
privileges because openshift-routes needs to be able to
create and read `Certificates`.
At the same time, the code complexity and LOC has been reduced.

Signed-off-by: Jack Henschel <[email protected]>
  • Loading branch information
jacksgt committed Feb 13, 2024
1 parent 5b9f2b3 commit 30617aa
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 289 deletions.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,14 @@ After modifying the source code, you can execute the tests with:
go test ./...
```

To run the controller locally, export the location of your kubeconfig file:

```sh
export KUBECONFIG=$HOME/path/to/kubeconfig
# adjust namespace as necessary
go run internal/cmd/main.go --namespace cert-manager --enable-leader-election=false
```

# Why is This a Separate Project?

We do not wish to support non Kubernetes (or kubernetes-sigs) APIs in cert-manager core. This adds
Expand Down
20 changes: 15 additions & 5 deletions internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ import (
"github.com/go-logr/logr"
routev1 "github.com/openshift/api/route/v1"
routev1client "github.com/openshift/client-go/route/clientset/versioned"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/builder"
Expand All @@ -35,9 +38,10 @@ import (
"github.com/cert-manager/openshift-routes/internal/cmd/app/options"
)

type Route struct {
type RouteController struct {
routeClient routev1client.Interface
certClient cmclient.Interface
coreClient corev1client.CoreV1Interface
eventRecorder record.EventRecorder

log logr.Logger
Expand Down Expand Up @@ -67,7 +71,7 @@ func shouldSync(log logr.Logger, route *routev1.Route) bool {
return false
}

func (r *Route) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
func (r *RouteController) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
log := r.log.WithValues("object", req.NamespacedName)
log.V(5).Info("started reconciling")
route, err := r.routeClient.RouteV1().Routes(req.Namespace).Get(ctx, req.Name, metav1.GetOptions{})
Expand All @@ -86,7 +90,7 @@ func (r *Route) Reconcile(ctx context.Context, req reconcile.Request) (reconcile
return r.sync(ctx, req, route.DeepCopy())
}

func New(base logr.Logger, config *rest.Config, recorder record.EventRecorder) (*Route, error) {
func New(base logr.Logger, config *rest.Config, recorder record.EventRecorder) (*RouteController, error) {
routeClient, err := routev1client.NewForConfig(config)
if err != nil {
return nil, err
Expand All @@ -95,10 +99,15 @@ func New(base logr.Logger, config *rest.Config, recorder record.EventRecorder) (
if err != nil {
return nil, err
}
clientset, err := kubernetes.NewForConfig(config)
if err != nil {
return nil, err
}

return &Route{
return &RouteController{
routeClient: routeClient,
certClient: certClient,
coreClient: clientset.CoreV1(),
log: base.WithName("route"),
eventRecorder: recorder,
}, nil
Expand All @@ -112,6 +121,7 @@ func AddToManager(mgr manager.Manager, opts *options.Options) error {
return builder.
ControllerManagedBy(mgr).
For(&routev1.Route{}).
Owns(&cmapi.CertificateRequest{}).
Owns(&cmapi.Certificate{}).
Owns(&corev1.Secret{}).
Complete(controller)
}
Loading

0 comments on commit 30617aa

Please sign in to comment.