From 25eefedc5fa2ab729234e242252e9be89db5cbc9 Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Fri, 1 Dec 2023 19:39:46 -0700 Subject: [PATCH 1/9] feat(helm): Add flag that customizes API access to secret resources --- .../command-cert-manager-issuer/README.md | 42 ++++++++++--------- .../templates/clusterrole.yaml | 8 ---- .../templates/deployment.yaml | 3 ++ .../templates/secretrole.yaml | 30 +++++++++++++ .../command-cert-manager-issuer/values.yaml | 10 +++++ docs/config_usage.markdown | 2 + docs/install.markdown | 42 +++++++++---------- .../certificaterequest_controller.go | 17 +++++--- internal/controllers/issuer_controller.go | 14 +++++-- main.go | 32 +++++++++----- 10 files changed, 131 insertions(+), 69 deletions(-) create mode 100644 deploy/charts/command-cert-manager-issuer/templates/secretrole.yaml diff --git a/deploy/charts/command-cert-manager-issuer/README.md b/deploy/charts/command-cert-manager-issuer/README.md index e992379..ef98db7 100644 --- a/deploy/charts/command-cert-manager-issuer/README.md +++ b/deploy/charts/command-cert-manager-issuer/README.md @@ -51,23 +51,25 @@ helm install command-cert-manager-issuer command-issuer/command-cert-manager-iss The following table lists the configurable parameters of the `command-cert-manager-issuer` chart and their default values. -| Parameter | Description | Default | -|-----------------------------------|-------------------------------------------------------|-------------------------------------------------------| -| `replicaCount` | Number of replica command-cert-manager-issuers to run | `1` | -| `image.repository` | Image repository | `ghcr.io/keyfactor/command-cert-manager-issuer` | -| `image.pullPolicy` | Image pull policy | `IfNotPresent` | -| `image.tag` | Image tag | `""` | -| `imagePullSecrets` | Image pull secrets | `[]` | -| `nameOverride` | Name override | `""` | -| `fullnameOverride` | Full name override | `""` | -| `crd.create` | Specifies if CRDs will be created | `true` | -| `crd.annotations` | Annotations to add to the CRD | `{}` | -| `serviceAccount.create` | Specifies if a service account should be created | `true` | -| `serviceAccount.annotations` | Annotations to add to the service account | `{}` | -| `serviceAccount.name` | Name of the service account to use | `""` (uses the fullname template if `create` is true) | -| `podAnnotations` | Annotations for the pod | `{}` | -| `podSecurityContext.runAsNonRoot` | Run pod as non-root | `true` | -| `securityContext` | Security context for the pod | `{}` (with commented out options) | -| `resources` | CPU/Memory resource requests/limits | `{}` (with commented out options) | -| `nodeSelector` | Node labels for pod assignment | `{}` | -| `tolerations` | Tolerations for pod assignment | `[]` | +| Parameter | Description | Default | +|----------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------|-------------------------------------------------------| +| `replicaCount` | Number of replica command-cert-manager-issuers to run | `1` | +| `image.repository` | Image repository | `ghcr.io/keyfactor/command-cert-manager-issuer` | +| `image.pullPolicy` | Image pull policy | `IfNotPresent` | +| `image.tag` | Image tag | `""` | +| `imagePullSecrets` | Image pull secrets | `[]` | +| `nameOverride` | Name override | `""` | +| `fullnameOverride` | Full name override | `""` | +| `crd.create` | Specifies if CRDs will be created | `true` | +| `crd.annotations` | Annotations to add to the CRD | `{}` | +| `serviceAccount.create` | Specifies if a service account should be created | `true` | +| `serviceAccount.annotations` | Annotations to add to the service account | `{}` | +| `serviceAccount.name` | Name of the service account to use | `""` (uses the fullname template if `create` is true) | +| `podAnnotations` | Annotations for the pod | `{}` | +| `podSecurityContext.runAsNonRoot` | Run pod as non-root | `true` | +| `securityContext` | Security context for the pod | `{}` (with commented out options) | +| `resources` | CPU/Memory resource requests/limits | `{}` (with commented out options) | +| `nodeSelector` | Node labels for pod assignment | `{}` | +| `tolerations` | Tolerations for pod assignment | `[]` | +| `secureMetrics.enabled` | Whether to enable and configure the kube-rbac-proxy sidecar for authorized and authenticated use of the /metrics endpoint by Prometheus. | `false` | +| `secretConfig.useClusterRoleForSecretAccess` | Specifies if the ServiceAccount should be granted access to the Secret resource using a ClusterRole | `false` | diff --git a/deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml b/deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml index c65a41b..489c4c1 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml @@ -5,14 +5,6 @@ metadata: {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} name: {{ include "command-cert-manager-issuer.name" . }}-manager-role rules: - - apiGroups: - - "" - resources: - - secrets - verbs: - - get - - list - - watch - apiGroups: - cert-manager.io resources: diff --git a/deploy/charts/command-cert-manager-issuer/templates/deployment.yaml b/deploy/charts/command-cert-manager-issuer/templates/deployment.yaml index 4eb16ce..a42935f 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/deployment.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/deployment.yaml @@ -55,6 +55,9 @@ spec: - --health-probe-bind-address=:8081 - --metrics-bind-address=127.0.0.1:8080 - --leader-elect + {{- if .Values.secretConfig.useClusterRoleForSecretAccess}} + - --secret-access-granted-at-cluster-level + {{- end}} command: - /manager image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" diff --git a/deploy/charts/command-cert-manager-issuer/templates/secretrole.yaml b/deploy/charts/command-cert-manager-issuer/templates/secretrole.yaml new file mode 100644 index 0000000..2ef0a3c --- /dev/null +++ b/deploy/charts/command-cert-manager-issuer/templates/secretrole.yaml @@ -0,0 +1,30 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: {{ if .Values.secretConfig.useClusterRoleForSecretAccess }}ClusterRole{{ else }}Role{{ end }} +metadata: + labels: + {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} + name: {{ include "command-cert-manager-issuer.name" . }}-secret-reader-role +rules: + - apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: {{ if .Values.secretConfig.useClusterRoleForSecretAccess }}ClusterRoleBinding{{ else }}RoleBinding{{ end }} +metadata: + labels: + {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} + name: {{ include "command-cert-manager-issuer.name" . }}-secret-reader-rolebinding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: {{ if .Values.secretConfig.useClusterRoleForSecretAccess }}ClusterRole{{ else }}Role{{ end }} + name: {{ include "command-cert-manager-issuer.name" . }}-secret-reader-role +subjects: + - kind: ServiceAccount + name: {{ include "command-cert-manager-issuer.serviceAccountName" . }} + namespace: {{ .Release.Namespace }} \ No newline at end of file diff --git a/deploy/charts/command-cert-manager-issuer/values.yaml b/deploy/charts/command-cert-manager-issuer/values.yaml index a521500..d3ed8e9 100644 --- a/deploy/charts/command-cert-manager-issuer/values.yaml +++ b/deploy/charts/command-cert-manager-issuer/values.yaml @@ -18,6 +18,16 @@ fullnameOverride: "" secureMetrics: enabled: false +secretConfig: + # If true, when using Issuer resources, the credential secret must be created in the same namespace as the + # Issuer resource. This access is facilitated by granting the ServiceAccount [get, list, watch] for the secret + # API at the cluster level. + # + # If false, both Issuer and ClusterIssuer must reference a secret in the same namespace as the chart/reconciler. + # This access is facilitated by granting the ServiceAccount [get, list, watch] for the secret API only for the + # namespace the chart is deployed in. + useClusterRoleForSecretAccess: true + crd: # Specifies whether CRDs will be created create: true diff --git a/docs/config_usage.markdown b/docs/config_usage.markdown index d5fbc73..7932855 100644 --- a/docs/config_usage.markdown +++ b/docs/config_usage.markdown @@ -98,6 +98,8 @@ data: EOF ``` +If the Helm chart was deployed with the `--set "secretConfig.useClusterRoleForSecretAccess=true"` flag, the secret must be created in the same namespace as any Issuer resources deployed. Otherwise, the secret must be created in the same namespace as the controller. + If the Command server is configured to use a self-signed certificate or with a certificate signed by an untrusted root, the CA certificate must be provided as a Kubernetes secret. ```shell kubectl -n command-issuer-system create secret generic command-ca-secret --from-file=ca.crt diff --git a/docs/install.markdown b/docs/install.markdown index af1b0bc..b985c79 100644 --- a/docs/install.markdown +++ b/docs/install.markdown @@ -91,29 +91,29 @@ The cert-manager external issuer for Keyfactor Command can also be installed usi # --set image.pullPolicy=Never # Only required if using a local image ``` - a. Modifications can be made by overriding the default values in the `values.yaml` file with the `--set` flag. For example, to override the `replicaCount` value, run the following command: - - ```shell - helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ - --namespace command-issuer-system \ - --create-namespace \ - --set image.repository=/keyfactor/command-cert-manager-issuer \ - --set image.tag= - --set replicaCount=2 - ``` - - b. Modifications can also be made by modifying the `values.yaml` file directly. For example, to override the + 1. Modifications can be made by overriding the default values in the `values.yaml` file with the `--set` flag. For example, to override the `replicaCount` value, run the following command: + + ```shell + helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ + --namespace command-issuer-system \ + --create-namespace \ + --set image.repository=/keyfactor/command-cert-manager-issuer \ + --set image.tag= + --set replicaCount=2 + ``` + + 2. Modifications can also be made by modifying the `values.yaml` file directly. For example, to override the `replicaCount` value, modify the `replicaCount` value in the `values.yaml` file: - ```yaml - cat < override.yaml - image: - repository: /keyfactor/command-cert-manager-issuer - pullPolicy: Never - tag: "latest" - replicaCount: 2 - EOF - ``` + ```yaml + cat < override.yaml + image: + repository: /keyfactor/command-cert-manager-issuer + pullPolicy: Never + tag: "latest" + replicaCount: 2 + EOF + ``` Then, use the `-f` flag to specify the `values.yaml` file: diff --git a/internal/controllers/certificaterequest_controller.go b/internal/controllers/certificaterequest_controller.go index 65c531a..4af9564 100644 --- a/internal/controllers/certificaterequest_controller.go +++ b/internal/controllers/certificaterequest_controller.go @@ -47,12 +47,12 @@ var ( type CertificateRequestReconciler struct { client.Client - Scheme *runtime.Scheme - SignerBuilder signer.CommandSignerBuilder - ClusterResourceNamespace string - - Clock clock.Clock - CheckApprovedCondition bool + Scheme *runtime.Scheme + SignerBuilder signer.CommandSignerBuilder + ClusterResourceNamespace string + SecretAccessGrantedAtClusterLevel bool + Clock clock.Clock + CheckApprovedCondition bool } // +kubebuilder:rbac:groups=cert-manager.io,resources=certificaterequests,verbs=get;list;watch @@ -192,6 +192,11 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, nil } + // If SecretAccessGrantedAtClusterLevel is false, we always look for the Secret in the same namespace as the Issuer + if !r.SecretAccessGrantedAtClusterLevel { + secretNamespace = r.ClusterResourceNamespace + } + // Get the Issuer or ClusterIssuer if err := r.Get(ctx, issuerName, issuer); err != nil { return ctrl.Result{}, fmt.Errorf("%w: %v", errGetIssuer, err) diff --git a/internal/controllers/issuer_controller.go b/internal/controllers/issuer_controller.go index 3094d52..b534379 100644 --- a/internal/controllers/issuer_controller.go +++ b/internal/controllers/issuer_controller.go @@ -48,10 +48,11 @@ var ( // IssuerReconciler reconciles a Issuer object type IssuerReconciler struct { client.Client - Kind string - ClusterResourceNamespace string - Scheme *runtime.Scheme - HealthCheckerBuilder signer.HealthCheckerBuilder + Kind string + ClusterResourceNamespace string + SecretAccessGrantedAtClusterLevel bool + Scheme *runtime.Scheme + HealthCheckerBuilder signer.HealthCheckerBuilder } //+kubebuilder:rbac:groups=command-issuer.keyfactor.com,resources=issuers;clusterissuers,verbs=get;list;watch @@ -119,6 +120,11 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res return ctrl.Result{}, nil } + // If SecretAccessGrantedAtClusterLevel is false, we always look for the Secret in the same namespace as the Issuer + if !r.SecretAccessGrantedAtClusterLevel { + authSecretName.Namespace = r.ClusterResourceNamespace + } + var authSecret corev1.Secret if err := r.Get(ctx, authSecretName, &authSecret); err != nil { return ctrl.Result{}, fmt.Errorf("%w, secret name: %s, reason: %v", errGetAuthSecret, authSecretName, err) diff --git a/main.go b/main.go index a7b2ddd..6e4d6e9 100644 --- a/main.go +++ b/main.go @@ -19,6 +19,7 @@ package main import ( "errors" "flag" + "fmt" "github.com/Keyfactor/command-issuer/internal/controllers" "github.com/Keyfactor/command-issuer/internal/issuer/signer" "github.com/Keyfactor/command-issuer/internal/issuer/util" @@ -62,6 +63,7 @@ func main() { var clusterResourceNamespace string var printVersion bool var disableApprovedCheck bool + var secretAccessGrantedAtClusterLevel bool flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") @@ -72,6 +74,8 @@ func main() { flag.BoolVar(&printVersion, "version", false, "Print version to stdout and exit") flag.BoolVar(&disableApprovedCheck, "disable-approved-check", false, "Disables waiting for CertificateRequests to have an approved condition before signing.") + flag.BoolVar(&secretAccessGrantedAtClusterLevel, "secret-access-granted-at-cluster-level", false, + "Set this flag to true if the secret access is granted at cluster level. This will allow the controller to access secrets in any namespace. ") opts := zap.Options{ Development: true, @@ -94,6 +98,12 @@ func main() { } } + if secretAccessGrantedAtClusterLevel { + setupLog.Info("expecting secret access at cluster level") + } else { + setupLog.Info(fmt.Sprintf("expecting secret access at namespace level (%s)", clusterResourceNamespace)) + } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, @@ -119,21 +129,23 @@ func main() { } if err = (&controllers.IssuerReconciler{ - Kind: "Issuer", - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - ClusterResourceNamespace: clusterResourceNamespace, - HealthCheckerBuilder: signer.CommandHealthCheckerFromIssuerAndSecretData, + Kind: "Issuer", + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ClusterResourceNamespace: clusterResourceNamespace, + SecretAccessGrantedAtClusterLevel: secretAccessGrantedAtClusterLevel, + HealthCheckerBuilder: signer.CommandHealthCheckerFromIssuerAndSecretData, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Issuer") os.Exit(1) } if err = (&controllers.IssuerReconciler{ - Kind: "ClusterIssuer", - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - ClusterResourceNamespace: clusterResourceNamespace, - HealthCheckerBuilder: signer.CommandHealthCheckerFromIssuerAndSecretData, + Kind: "ClusterIssuer", + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ClusterResourceNamespace: clusterResourceNamespace, + SecretAccessGrantedAtClusterLevel: secretAccessGrantedAtClusterLevel, + HealthCheckerBuilder: signer.CommandHealthCheckerFromIssuerAndSecretData, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterIssuer") os.Exit(1) From 6a9bf0a05990acb490df5184baf008ef9099b810 Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Mon, 4 Dec 2023 09:58:06 -0700 Subject: [PATCH 2/9] chore(values): Change default for useClusterRoleForSecretAccess to false --- deploy/charts/command-cert-manager-issuer/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy/charts/command-cert-manager-issuer/values.yaml b/deploy/charts/command-cert-manager-issuer/values.yaml index d3ed8e9..4806cc0 100644 --- a/deploy/charts/command-cert-manager-issuer/values.yaml +++ b/deploy/charts/command-cert-manager-issuer/values.yaml @@ -26,7 +26,7 @@ secretConfig: # If false, both Issuer and ClusterIssuer must reference a secret in the same namespace as the chart/reconciler. # This access is facilitated by granting the ServiceAccount [get, list, watch] for the secret API only for the # namespace the chart is deployed in. - useClusterRoleForSecretAccess: true + useClusterRoleForSecretAccess: false crd: # Specifies whether CRDs will be created From 0cc5920e59491f636ba08334425331d7271ef1aa Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Tue, 5 Dec 2023 12:38:29 -0700 Subject: [PATCH 3/9] chore(docs): Refactor docs to reflect addition of secretConfig.useClusterRoleForSecretAccess value --- .../command-cert-manager-issuer/README.md | 32 +++++++++++---- docs/config_usage.markdown | 3 +- docs/install.markdown | 41 ++++++++++--------- .../certificaterequest_controller_test.go | 13 +++--- .../controllers/issuer_controller_test.go | 11 ++--- main.go | 13 +++--- 6 files changed, 66 insertions(+), 47 deletions(-) diff --git a/deploy/charts/command-cert-manager-issuer/README.md b/deploy/charts/command-cert-manager-issuer/README.md index ef98db7..6f625cb 100644 --- a/deploy/charts/command-cert-manager-issuer/README.md +++ b/deploy/charts/command-cert-manager-issuer/README.md @@ -18,32 +18,46 @@ The Command external issuer for cert-manager allows users to enroll certificates ### Add Helm Repository -```bash +```shell helm repo add command-issuer https://keyfactor.github.io/command-cert-manager-issuer helm repo update ``` ### Install Chart -```bash -helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer +```shell +helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ + --namespace command-issuer-system \ + --create-namespace \ + --set image.repository=/keyfactor/command-cert-manager-issuer \ + --set image.tag= \ + --set crd.create=true \ + # --set image.pullPolicy=Never # Only required if using a local image ``` -Modifications can be made by overriding the default values in the `values.yaml` file with the `--set` flag. For example, to override the `replicaCount` value, run the following command: -```bash +Modifications can be made by overriding the default values in the `values.yaml` file with the `--set` flag. For example, to override the `secretConfig.useClusterRoleForSecretAccess` to configure the chart to use a cluster role for secret access, run the following command: + +```shell helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ - --set replicaCount=2 + --namespace command-issuer-system \ + --create-namespace \ + --set image.repository=/keyfactor/command-cert-manager-issuer \ + --set image.tag= \ + --set crd.create=true \ + --set secretConfig.useClusterRoleForSecretAccess=true ``` -Modifications can also be made by modifying the `values.yaml` file directly. For example, to override the `replicaCount` value, modify the `replicaCount` value in the `values.yaml` file: +Modifications can also be made by modifying the `values.yaml` file directly. For example, to override the `secretConfig.useClusterRoleForSecretAccess` value to configure the chart to use a cluster role for secret access, modify the `secretConfig.useClusterRoleForSecretAccess` value in the `values.yaml` file by creating an override file: ```yaml cat < override.yaml -replicaCount: 2 +secretConfig: + useClusterRoleForSecretAccess: true EOF ``` Then, use the `-f` flag to specify the `values.yaml` file: -```bash +```shell helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ + --namespace command-issuer-system \ -f override.yaml ``` diff --git a/docs/config_usage.markdown b/docs/config_usage.markdown index 7932855..a40cd2d 100644 --- a/docs/config_usage.markdown +++ b/docs/config_usage.markdown @@ -83,6 +83,7 @@ kfutil import --metadata --file metadata.json ### Authentication Authentication to the Command platform is done using basic authentication. The credentials must be provided as a Kubernetes `kubernetes.io/basic-auth` secret. These credentials should be for a user with "Certificate Enrollment: Enroll CSR" and "API: Read" permissions in Command. +If the Helm chart was deployed with the `--set "secretConfig.useClusterRoleForSecretAccess=true"` flag, the secret must be created in the same namespace as any Issuer resources deployed. Otherwise, the secret must be created in the same namespace as the controller. Create a `kubernetes.io/basic-auth` secret with the Keyfactor Command username and password: ```shell @@ -98,8 +99,6 @@ data: EOF ``` -If the Helm chart was deployed with the `--set "secretConfig.useClusterRoleForSecretAccess=true"` flag, the secret must be created in the same namespace as any Issuer resources deployed. Otherwise, the secret must be created in the same namespace as the controller. - If the Command server is configured to use a self-signed certificate or with a certificate signed by an untrusted root, the CA certificate must be provided as a Kubernetes secret. ```shell kubectl -n command-issuer-system create secret generic command-ca-secret --from-file=ca.crt diff --git a/docs/install.markdown b/docs/install.markdown index b985c79..9fc4303 100644 --- a/docs/install.markdown +++ b/docs/install.markdown @@ -42,7 +42,7 @@ kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/ The cert-manager external issuer for Keyfactor Command is distributed as source code, and the container must be built manually. The container image can be built using the following command: ```shell -make docker-build DOCKER_REGISTRY= DOCKER_IMAGE_NAME=keyfactor/command-cert-manager-issuer +make docker-build DOCKER_REGISTRY= DOCKER_IMAGE_NAME=keyfactor/command-cert-manager-issuer VERSION= ``` ###### :pushpin: The container image can be built using Docker Buildx by running `make docker-buildx`. This will build the image for all supported platforms. @@ -50,7 +50,7 @@ make docker-build DOCKER_REGISTRY= DOCKER_IMAGE_NAME=ke To push the container image to a container registry, run the following command: ```shell docker login -make docker-push DOCKER_REGISTRY= DOCKER_IMAGE_NAME=keyfactor/command-cert-manager-issuer +make docker-push DOCKER_REGISTRY= DOCKER_IMAGE_NAME=keyfactor/command-cert-manager-issuer VERSION= ``` ### Installation from Manifests @@ -66,7 +66,7 @@ The cert-manager external issuer for Keyfactor Command can be installed using th 2. Finally, deploy the controller to the cluster: ```shell - make deploy DOCKER_REGISTRY= DOCKER_IMAGE_NAME=keyfactor/command-cert-manager-issuer + make deploy DOCKER_REGISTRY= DOCKER_IMAGE_NAME=keyfactor/command-cert-manager-issuer VERSION= ``` ### Installation from Helm Chart @@ -75,51 +75,54 @@ The cert-manager external issuer for Keyfactor Command can also be installed usi 1. Add the Helm repository: - ```bash + ```shell helm repo add command-issuer https://keyfactor.github.io/command-cert-manager-issuer helm repo update ``` 2. Then, install the chart: - ```bash + ```shell helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ --namespace command-issuer-system \ --create-namespace \ --set image.repository=/keyfactor/command-cert-manager-issuer \ - --set image.tag= + --set image.tag= \ + --set crd.create=true \ # --set image.pullPolicy=Never # Only required if using a local image ``` - 1. Modifications can be made by overriding the default values in the `values.yaml` file with the `--set` flag. For example, to override the `replicaCount` value, run the following command: + 1. Modifications can be made by overriding the default values in the `values.yaml` file with the `--set` flag. For example, to override the `secretConfig.useClusterRoleForSecretAccess` to configure the chart to use a cluster role for secret access, run the following command: ```shell helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ --namespace command-issuer-system \ --create-namespace \ --set image.repository=/keyfactor/command-cert-manager-issuer \ - --set image.tag= - --set replicaCount=2 + --set image.tag= \ + --set crd.create=true \ + --set secretConfig.useClusterRoleForSecretAccess=true ``` - 2. Modifications can also be made by modifying the `values.yaml` file directly. For example, to override the - `replicaCount` value, modify the `replicaCount` value in the `values.yaml` file: + 2. Modifications can also be made by modifying the `values.yaml` file directly. For example, to override the `secretConfig.useClusterRoleForSecretAccess` value to configure the chart to use a cluster role for secret access, modify the `secretConfig.useClusterRoleForSecretAccess` value in the `values.yaml` file by creating an override file: ```yaml cat < override.yaml image: repository: /keyfactor/command-cert-manager-issuer pullPolicy: Never - tag: "latest" - replicaCount: 2 + tag: "" + secretConfig: + useClusterRoleForSecretAccess: true EOF ``` - Then, use the `-f` flag to specify the `values.yaml` file: - - ```yaml - helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ - -f override.yaml - ``` + Then, use the `-f` flag to specify the `values.yaml` file: + + ```shell + helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ + --namespace command-issuer-system \ + -f override.yaml + ``` Next, complete the [Usage](config_usage.markdown) steps to configure the cert-manager external issuer for Keyfactor Command. diff --git a/internal/controllers/certificaterequest_controller_test.go b/internal/controllers/certificaterequest_controller_test.go index f113031..1589701 100644 --- a/internal/controllers/certificaterequest_controller_test.go +++ b/internal/controllers/certificaterequest_controller_test.go @@ -602,12 +602,13 @@ func TestCertificateRequestReconcile(t *testing.T) { WithObjects(tc.objects...). Build() controller := CertificateRequestReconciler{ - Client: fakeClient, - Scheme: scheme, - ClusterResourceNamespace: tc.clusterResourceNamespace, - SignerBuilder: tc.Builder, - CheckApprovedCondition: true, - Clock: fixedClock, + Client: fakeClient, + Scheme: scheme, + ClusterResourceNamespace: tc.clusterResourceNamespace, + SignerBuilder: tc.Builder, + CheckApprovedCondition: true, + Clock: fixedClock, + SecretAccessGrantedAtClusterLevel: true, } result, err := controller.Reconcile( ctrl.LoggerInto(context.TODO(), logrtesting.NewTestLogger(t)), diff --git a/internal/controllers/issuer_controller_test.go b/internal/controllers/issuer_controller_test.go index 91f2a8e..f086ea5 100644 --- a/internal/controllers/issuer_controller_test.go +++ b/internal/controllers/issuer_controller_test.go @@ -251,11 +251,12 @@ func TestIssuerReconcile(t *testing.T) { tc.kind = "Issuer" } controller := IssuerReconciler{ - Kind: tc.kind, - Client: fakeClient, - Scheme: scheme, - HealthCheckerBuilder: tc.healthCheckerBuilder, - ClusterResourceNamespace: tc.clusterResourceNamespace, + Kind: tc.kind, + Client: fakeClient, + Scheme: scheme, + HealthCheckerBuilder: tc.healthCheckerBuilder, + ClusterResourceNamespace: tc.clusterResourceNamespace, + SecretAccessGrantedAtClusterLevel: true, } result, err := controller.Reconcile( ctrl.LoggerInto(context.TODO(), logrtesting.NewTestLogger(t)), diff --git a/main.go b/main.go index 6e4d6e9..b1d9b91 100644 --- a/main.go +++ b/main.go @@ -151,12 +151,13 @@ func main() { os.Exit(1) } if err = (&controllers.CertificateRequestReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - ClusterResourceNamespace: clusterResourceNamespace, - SignerBuilder: signer.CommandSignerFromIssuerAndSecretData, - CheckApprovedCondition: !disableApprovedCheck, - Clock: clock.RealClock{}, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ClusterResourceNamespace: clusterResourceNamespace, + SignerBuilder: signer.CommandSignerFromIssuerAndSecretData, + CheckApprovedCondition: !disableApprovedCheck, + SecretAccessGrantedAtClusterLevel: secretAccessGrantedAtClusterLevel, + Clock: clock.RealClock{}, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "CertificateRequest") os.Exit(1) From 40fefb726650c575599502ad32a79c89916fb908 Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Tue, 5 Dec 2023 13:39:52 -0700 Subject: [PATCH 4/9] chore(meta): Add description to meta creation --- docs/config_usage.markdown | 2 +- internal/controllers/certificaterequest_controller.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/config_usage.markdown b/docs/config_usage.markdown index a40cd2d..6e1c299 100644 --- a/docs/config_usage.markdown +++ b/docs/config_usage.markdown @@ -27,7 +27,7 @@ cat <> metadata.json { "AllowAPI": true, "DataType": 1, - "Description": "The namespace that the issuer resource was created in.", + "Description": "The namespace that the issuer resource was created in that .", "Name": "Issuer-Namespace" }, { diff --git a/internal/controllers/certificaterequest_controller.go b/internal/controllers/certificaterequest_controller.go index 4af9564..c506c7e 100644 --- a/internal/controllers/certificaterequest_controller.go +++ b/internal/controllers/certificaterequest_controller.go @@ -185,6 +185,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R case *commandissuer.ClusterIssuer: secretNamespace = r.ClusterResourceNamespace log = log.WithValues("clusterissuer", issuerName) + meta.ControllerKind = "clusterissuer" default: err := fmt.Errorf("unexpected issuer type: %v", t) log.Error(err, "The issuerRef referred to a registered Kind which is not yet handled. Ignoring.") From cfa09560de3430f8b276eff55e6d4cda4709945b Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Wed, 6 Dec 2023 15:58:24 -0700 Subject: [PATCH 5/9] fix(config): Implement K8s client-go in secondary API client for secret/config retrieval --- go.mod | 2 +- .../certificaterequest_controller.go | 12 +- .../certificaterequest_controller_test.go | 5 +- .../controllers/fake_configclient_test.go | 57 +++++++ internal/controllers/issuer_controller.go | 10 +- .../controllers/issuer_controller_test.go | 7 +- internal/issuer/util/configclient.go | 152 ++++++++++++++++++ internal/issuer/util/configclient_test.go | 88 ++++++++++ main.go | 10 ++ 9 files changed, 330 insertions(+), 13 deletions(-) create mode 100644 internal/controllers/fake_configclient_test.go create mode 100644 internal/issuer/util/configclient.go create mode 100644 internal/issuer/util/configclient_test.go diff --git a/go.mod b/go.mod index 08309cf..3d54f23 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( k8s.io/api v0.26.3 k8s.io/apimachinery v0.26.3 k8s.io/client-go v0.26.3 + k8s.io/klog/v2 v2.90.1 k8s.io/utils v0.0.0-20230313181309-38a27ef9d749 sigs.k8s.io/controller-runtime v0.14.6 ) @@ -72,7 +73,6 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apiextensions-apiserver v0.26.3 // indirect k8s.io/component-base v0.26.3 // indirect - k8s.io/klog/v2 v2.90.1 // indirect k8s.io/kube-aggregator v0.26.3 // indirect k8s.io/kube-openapi v0.0.0-20230327201221-f5883ff37f0c // indirect sigs.k8s.io/gateway-api v0.6.2 // indirect diff --git a/internal/controllers/certificaterequest_controller.go b/internal/controllers/certificaterequest_controller.go index c506c7e..ab4d84f 100644 --- a/internal/controllers/certificaterequest_controller.go +++ b/internal/controllers/certificaterequest_controller.go @@ -47,6 +47,7 @@ var ( type CertificateRequestReconciler struct { client.Client + ConfigClient issuerutil.ConfigClient Scheme *runtime.Scheme SignerBuilder signer.CommandSignerBuilder ClusterResourceNamespace string @@ -156,8 +157,8 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R // Add a Ready condition if one does not already exist if ready := cmutil.GetCertificateRequestCondition(&certificateRequest, cmapi.CertificateRequestConditionReady); ready == nil { - log.Info("Initialising Ready condition") - setReadyCondition(cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Initialising") + log.Info("Initializing Ready condition") + setReadyCondition(cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Initializing") return ctrl.Result{}, nil } @@ -214,13 +215,16 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, errIssuerNotReady } + // Set the context on the config client + r.ConfigClient.SetContext(ctx) + authSecretName := types.NamespacedName{ Name: issuerSpec.SecretName, Namespace: secretNamespace, } var authSecret corev1.Secret - if err := r.Get(ctx, authSecretName, &authSecret); err != nil { + if err = r.ConfigClient.GetSecret(authSecretName, &authSecret); err != nil { return ctrl.Result{}, fmt.Errorf("%w, secret name: %s, reason: %v", errGetAuthSecret, authSecretName, err) } @@ -233,7 +237,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R var caSecret corev1.Secret if issuerSpec.CaSecretName != "" { // If the CA secret name is not specified, we will not attempt to retrieve it - err = r.Get(ctx, caSecretName, &caSecret) + err = r.ConfigClient.GetSecret(caSecretName, &caSecret) if err != nil { return ctrl.Result{}, fmt.Errorf("%w, secret name: %s, reason: %v", errGetCaSecret, caSecretName, err) } diff --git a/internal/controllers/certificaterequest_controller_test.go b/internal/controllers/certificaterequest_controller_test.go index 1589701..075cb03 100644 --- a/internal/controllers/certificaterequest_controller_test.go +++ b/internal/controllers/certificaterequest_controller_test.go @@ -23,7 +23,7 @@ import ( cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" cmgen "github.com/cert-manager/cert-manager/test/unit/gen" - logrtesting "github.com/go-logr/logr/testing" + logrtesting "github.com/go-logr/logr/testr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -603,6 +603,7 @@ func TestCertificateRequestReconcile(t *testing.T) { Build() controller := CertificateRequestReconciler{ Client: fakeClient, + ConfigClient: NewFakeConfigClient(fakeClient), Scheme: scheme, ClusterResourceNamespace: tc.clusterResourceNamespace, SignerBuilder: tc.Builder, @@ -611,7 +612,7 @@ func TestCertificateRequestReconcile(t *testing.T) { SecretAccessGrantedAtClusterLevel: true, } result, err := controller.Reconcile( - ctrl.LoggerInto(context.TODO(), logrtesting.NewTestLogger(t)), + ctrl.LoggerInto(context.TODO(), logrtesting.New(t)), reconcile.Request{NamespacedName: tc.name}, ) if tc.expectedError != nil { diff --git a/internal/controllers/fake_configclient_test.go b/internal/controllers/fake_configclient_test.go new file mode 100644 index 0000000..e97a02e --- /dev/null +++ b/internal/controllers/fake_configclient_test.go @@ -0,0 +1,57 @@ +/* +Copyright 2023 The Keyfactor Command Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "github.com/Keyfactor/command-issuer/internal/issuer/util" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// FakeConfigClient is a fake implementation of the util.ConfigClient interface +// It forwards requests destined for the Kubernetes API server implemented by +// the util.ConfigClient interface to a fake Kubernetes API server implemented +// by the client.Client interface. + +// Force the compiler to check that FakeConfigClient implements the util.ConfigClient interface +var _ util.ConfigClient = &FakeConfigClient{} + +type FakeConfigClient struct { + client client.Client + ctx context.Context +} + +// NewFakeConfigClient uses the +func NewFakeConfigClient(fakeControllerRuntimeClient client.Client) util.ConfigClient { + return &FakeConfigClient{ + client: fakeControllerRuntimeClient, + } +} + +func (f FakeConfigClient) SetContext(ctx context.Context) { + f.ctx = ctx +} + +func (f FakeConfigClient) GetConfigMap(name types.NamespacedName, out *corev1.ConfigMap) error { + return f.client.Get(f.ctx, name, out) +} + +func (f FakeConfigClient) GetSecret(name types.NamespacedName, out *corev1.Secret) error { + return f.client.Get(f.ctx, name, out) +} diff --git a/internal/controllers/issuer_controller.go b/internal/controllers/issuer_controller.go index b534379..10a0809 100644 --- a/internal/controllers/issuer_controller.go +++ b/internal/controllers/issuer_controller.go @@ -48,6 +48,7 @@ var ( // IssuerReconciler reconciles a Issuer object type IssuerReconciler struct { client.Client + ConfigClient issuerutil.ConfigClient Kind string ClusterResourceNamespace string SecretAccessGrantedAtClusterLevel bool @@ -73,7 +74,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res issuer, err := r.newIssuer() if err != nil { - log.Error(err, "Unrecognised issuer type") + log.Error(err, "Unrecognized issuer type") return ctrl.Result{}, nil } if err := r.Get(ctx, req.NamespacedName, issuer); err != nil { @@ -125,8 +126,11 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res authSecretName.Namespace = r.ClusterResourceNamespace } + // Set the context on the config client + r.ConfigClient.SetContext(ctx) + var authSecret corev1.Secret - if err := r.Get(ctx, authSecretName, &authSecret); err != nil { + if err := r.ConfigClient.GetSecret(authSecretName, &authSecret); err != nil { return ctrl.Result{}, fmt.Errorf("%w, secret name: %s, reason: %v", errGetAuthSecret, authSecretName, err) } @@ -139,7 +143,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res var caSecret corev1.Secret if issuerSpec.CaSecretName != "" { // If the CA secret name is not specified, we will not attempt to retrieve it - err = r.Get(ctx, caSecretName, &caSecret) + err = r.ConfigClient.GetSecret(caSecretName, &caSecret) if err != nil { return ctrl.Result{}, fmt.Errorf("%w, secret name: %s, reason: %v", errGetCaSecret, caSecretName, err) } diff --git a/internal/controllers/issuer_controller_test.go b/internal/controllers/issuer_controller_test.go index f086ea5..12c2c54 100644 --- a/internal/controllers/issuer_controller_test.go +++ b/internal/controllers/issuer_controller_test.go @@ -21,7 +21,7 @@ import ( "errors" "github.com/Keyfactor/command-issuer/internal/issuer/signer" issuerutil "github.com/Keyfactor/command-issuer/internal/issuer/util" - logrtesting "github.com/go-logr/logr/testing" + logrtesting "github.com/go-logr/logr/testr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -126,7 +126,7 @@ func TestIssuerReconcile(t *testing.T) { expectedReadyConditionStatus: commandissuer.ConditionTrue, expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval}, }, - "issuer-kind-unrecognised": { + "issuer-kind-Unrecognized": { kind: "UnrecognizedType", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, }, @@ -253,13 +253,14 @@ func TestIssuerReconcile(t *testing.T) { controller := IssuerReconciler{ Kind: tc.kind, Client: fakeClient, + ConfigClient: NewFakeConfigClient(fakeClient), Scheme: scheme, HealthCheckerBuilder: tc.healthCheckerBuilder, ClusterResourceNamespace: tc.clusterResourceNamespace, SecretAccessGrantedAtClusterLevel: true, } result, err := controller.Reconcile( - ctrl.LoggerInto(context.TODO(), logrtesting.NewTestLogger(t)), + ctrl.LoggerInto(context.TODO(), logrtesting.New(t)), reconcile.Request{NamespacedName: tc.name}, ) if tc.expectedError != nil { diff --git a/internal/issuer/util/configclient.go b/internal/issuer/util/configclient.go new file mode 100644 index 0000000..f8f9944 --- /dev/null +++ b/internal/issuer/util/configclient.go @@ -0,0 +1,152 @@ +/* +Copyright 2023 The Keyfactor Command Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "context" + "fmt" + authv1 "k8s.io/api/authorization/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes" + "k8s.io/klog/v2" + ctrl "sigs.k8s.io/controller-runtime" +) + +type ConfigClient interface { + SetContext(ctx context.Context) + GetConfigMap(name types.NamespacedName, out *corev1.ConfigMap) error + GetSecret(name types.NamespacedName, out *corev1.Secret) error +} + +type configClient struct { + ctx context.Context + logger klog.Logger + client kubernetes.Interface + accessCache map[string]bool + + verifyAccessFunc func(apiResource string, resource types.NamespacedName) error +} + +func NewConfigClient(ctx context.Context) (ConfigClient, error) { + config := ctrl.GetConfigOrDie() + + // Create the clientset + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + return nil, fmt.Errorf("failed to create clientset: %w", err) + } + + client := &configClient{ + client: clientset, + accessCache: make(map[string]bool), + ctx: ctx, + logger: klog.NewKlogr(), + } + + client.verifyAccessFunc = client.verifyAccessToResource + + return client, nil +} + +func (c *configClient) SetContext(ctx context.Context) { + c.ctx = ctx + c.logger = klog.FromContext(ctx) +} + +func (c *configClient) verifyAccessToResource(apiResource string, resource types.NamespacedName) error { + verbs := []string{"get", "list", "watch"} + + for _, verb := range verbs { + ssar := &authv1.SelfSubjectAccessReview{ + Spec: authv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authv1.ResourceAttributes{ + Name: resource.Name, + Namespace: resource.Namespace, + + Group: "", + Resource: apiResource, + Verb: verb, + }, + }, + } + + ssar, err := c.client.AuthorizationV1().SelfSubjectAccessReviews().Create(c.ctx, ssar, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("failed to create SelfSubjectAccessReview to check access to %s for verb %q: %w", apiResource, verb, err) + } + + if !ssar.Status.Allowed { + return fmt.Errorf("client does not have access to %s called %q for verb %q, reason: %v", apiResource, resource.String(), verb, ssar.Status.String()) + } + } + + c.logger.Info(fmt.Sprintf("Client has access to %s called %q", apiResource, resource.String())) + + return nil +} + +func (c *configClient) GetConfigMap(name types.NamespacedName, out *corev1.ConfigMap) error { + if c == nil { + return fmt.Errorf("config client is nil") + } + + // Check if the client has access to the configmap resource + if ok, _ := c.accessCache[name.String()]; !ok { + err := c.verifyAccessFunc("configmaps", name) + if err != nil { + return err + } + c.accessCache[name.String()] = true + } + + // Get the configmap + configmap, err := c.client.CoreV1().ConfigMaps(name.Namespace).Get(c.ctx, name.Name, metav1.GetOptions{}) + if err != nil { + return err + } + + // Copy the configmap into the out parameter + configmap.DeepCopyInto(out) + return nil +} + +func (c *configClient) GetSecret(name types.NamespacedName, out *corev1.Secret) error { + if c == nil { + return fmt.Errorf("config client is nil") + } + + // Check if the client has access to the secret resource + if ok, _ := c.accessCache[name.String()]; !ok { + err := c.verifyAccessFunc("secrets", name) + if err != nil { + return err + } + c.accessCache[name.String()] = true + } + + // Get the secret + secret, err := c.client.CoreV1().Secrets(name.Namespace).Get(c.ctx, name.Name, metav1.GetOptions{}) + if err != nil { + return err + } + + // Copy the secret into the out parameter + secret.DeepCopyInto(out) + return nil +} diff --git a/internal/issuer/util/configclient_test.go b/internal/issuer/util/configclient_test.go new file mode 100644 index 0000000..e57f922 --- /dev/null +++ b/internal/issuer/util/configclient_test.go @@ -0,0 +1,88 @@ +/* +Copyright 2023 The Keyfactor Command Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "context" + logrtesting "github.com/go-logr/logr/testr" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/fake" + ctrl "sigs.k8s.io/controller-runtime" + "testing" +) + +func TestConfigClient(t *testing.T) { + var err error + + // Define namespaced names for test objects + configMapName := types.NamespacedName{Name: "test-configmap", Namespace: "default"} + secretName := types.NamespacedName{Name: "test-secret", Namespace: "default"} + + // Create and inject fake ConfigMap + testConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: configMapName.Name, Namespace: configMapName.Namespace}, + } + + // Create and inject fake Secret + testSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName.Name, Namespace: secretName.Namespace}, + } + + // Create a fake clientset with the test objects + clientset := fake.NewSimpleClientset([]runtime.Object{ + testConfigMap, + testSecret, + }...) + + // We can't test NewConfigClient unless we can mock ctrl.GetConfigOrDie() and kubernetes.NewForConfig() + // So we'll just test the methods that use the clientset + + // Create a ConfigClient + client := &configClient{ + client: clientset, + accessCache: make(map[string]bool), + } + + // The fake client doesn't implement authorization.k8s.io/v1 SelfSubjectAccessReview + // So we'll mock the verifyAccessFunc + client.verifyAccessFunc = func(apiResource string, resource types.NamespacedName) error { + return nil + } + + // Setup logging for test environment by setting the context + client.SetContext(ctrl.LoggerInto(context.TODO(), logrtesting.New(t))) + + t.Run("GetConfigMap", func(t *testing.T) { + // Test GetConfigMap + var out corev1.ConfigMap + err = client.GetConfigMap(configMapName, &out) + assert.NoError(t, err) + assert.Equal(t, testConfigMap, &out) + }) + + t.Run("GetSecret", func(t *testing.T) { + // Test GetSecret + var out corev1.Secret + err = client.GetSecret(secretName, &out) + assert.NoError(t, err) + assert.Equal(t, testSecret, &out) + }) +} diff --git a/main.go b/main.go index b1d9b91..88fda9c 100644 --- a/main.go +++ b/main.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "context" "errors" "flag" "fmt" @@ -104,6 +105,12 @@ func main() { setupLog.Info(fmt.Sprintf("expecting secret access at namespace level (%s)", clusterResourceNamespace)) } + ctx := context.Background() + configClient, err := util.NewConfigClient(ctx) + if err != nil { + setupLog.Error(err, "error creating config client") + } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, @@ -131,6 +138,7 @@ func main() { if err = (&controllers.IssuerReconciler{ Kind: "Issuer", Client: mgr.GetClient(), + ConfigClient: configClient, Scheme: mgr.GetScheme(), ClusterResourceNamespace: clusterResourceNamespace, SecretAccessGrantedAtClusterLevel: secretAccessGrantedAtClusterLevel, @@ -142,6 +150,7 @@ func main() { if err = (&controllers.IssuerReconciler{ Kind: "ClusterIssuer", Client: mgr.GetClient(), + ConfigClient: configClient, Scheme: mgr.GetScheme(), ClusterResourceNamespace: clusterResourceNamespace, SecretAccessGrantedAtClusterLevel: secretAccessGrantedAtClusterLevel, @@ -153,6 +162,7 @@ func main() { if err = (&controllers.CertificateRequestReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), + ConfigClient: configClient, ClusterResourceNamespace: clusterResourceNamespace, SignerBuilder: signer.CommandSignerFromIssuerAndSecretData, CheckApprovedCondition: !disableApprovedCheck, From 8fd8dea3ceb35e7274c50bf08190a82c7deed627 Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Wed, 6 Dec 2023 16:05:42 -0700 Subject: [PATCH 6/9] fix(lint): Run linters --- internal/controllers/fake_configclient_test.go | 6 +++--- internal/issuer/util/configclient.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/controllers/fake_configclient_test.go b/internal/controllers/fake_configclient_test.go index e97a02e..6143d36 100644 --- a/internal/controllers/fake_configclient_test.go +++ b/internal/controllers/fake_configclient_test.go @@ -44,14 +44,14 @@ func NewFakeConfigClient(fakeControllerRuntimeClient client.Client) util.ConfigC } } -func (f FakeConfigClient) SetContext(ctx context.Context) { +func (f *FakeConfigClient) SetContext(ctx context.Context) { f.ctx = ctx } -func (f FakeConfigClient) GetConfigMap(name types.NamespacedName, out *corev1.ConfigMap) error { +func (f *FakeConfigClient) GetConfigMap(name types.NamespacedName, out *corev1.ConfigMap) error { return f.client.Get(f.ctx, name, out) } -func (f FakeConfigClient) GetSecret(name types.NamespacedName, out *corev1.Secret) error { +func (f *FakeConfigClient) GetSecret(name types.NamespacedName, out *corev1.Secret) error { return f.client.Get(f.ctx, name, out) } diff --git a/internal/issuer/util/configclient.go b/internal/issuer/util/configclient.go index f8f9944..990e9a1 100644 --- a/internal/issuer/util/configclient.go +++ b/internal/issuer/util/configclient.go @@ -107,7 +107,7 @@ func (c *configClient) GetConfigMap(name types.NamespacedName, out *corev1.Confi } // Check if the client has access to the configmap resource - if ok, _ := c.accessCache[name.String()]; !ok { + if _, ok := c.accessCache[name.String()]; !ok { err := c.verifyAccessFunc("configmaps", name) if err != nil { return err @@ -132,7 +132,7 @@ func (c *configClient) GetSecret(name types.NamespacedName, out *corev1.Secret) } // Check if the client has access to the secret resource - if ok, _ := c.accessCache[name.String()]; !ok { + if _, ok := c.accessCache[name.String()]; !ok { err := c.verifyAccessFunc("secrets", name) if err != nil { return err From 579442e40ab152f771667fdcd543aad06ade2b97 Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Wed, 6 Dec 2023 17:12:57 -0700 Subject: [PATCH 7/9] chore(changelog): Update changelog --- CHANGELOG.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aac1554..b7817fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,4 +8,13 @@ * fix(helm): CRDs now correspond to correct values for the `command-issuer`. * fix(helm): Signer Helm Chart now includes a `secureMetrics` value to enable/disable sidecar RBAC container for further protection of the `/metrics` endpoint. * fix(signer): Signer now returns CA chain bytes instead of appending to the leaf certificate. -* fix(role): Removed permissions for `configmaps` resource types for the `leader-election-role` role. \ No newline at end of file +* fix(role): Removed permissions for `configmaps` resource types for the `leader-election-role` role. + +# v1.0.5 + +## Features +* feat(controller): Implement Kubernetes `client-go` REST client for Secret/ConfigMap retrieval to bypass `controller-runtime` caching system. This enables the reconciler to retrieve Secret and ConfigMap resources at the namespace scope with only namespace-level permissions. + +## Fixes +* fix(helm): Add configuration flag to configure chart to either grant cluster-scoped or namespace-scoped access to Secret and ConfigMap API +* fix(controller): Add logic to read secret from reconciler namespace or Issuer namespace depending on Helm configuration. From 18281263c73949ed9fae0ff334d713d97fcedcb9 Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Thu, 14 Dec 2023 11:26:16 -0700 Subject: [PATCH 8/9] chore(comments): Write function comments and update license header --- api/v1alpha1/clusterissuer_types.go | 2 +- api/v1alpha1/groupversion_info.go | 2 +- api/v1alpha1/issuer_types.go | 2 +- .../controllers/certificaterequest_controller.go | 2 ++ .../certificaterequest_controller_test.go | 2 +- internal/controllers/fake_configclient_test.go | 2 +- internal/controllers/issuer_controller.go | 5 +++-- internal/controllers/issuer_controller_test.go | 2 +- internal/controllers/suite_test.go | 2 +- internal/issuer/signer/signer.go | 14 ++++++++++++++ internal/issuer/signer/signer_test.go | 2 +- internal/issuer/util/configclient.go | 14 +++++++++++++- internal/issuer/util/configclient_test.go | 2 +- internal/issuer/util/util.go | 6 +++++- main.go | 2 +- 15 files changed, 47 insertions(+), 14 deletions(-) diff --git a/api/v1alpha1/clusterissuer_types.go b/api/v1alpha1/clusterissuer_types.go index a4b83a1..7df7fd5 100644 --- a/api/v1alpha1/clusterissuer_types.go +++ b/api/v1alpha1/clusterissuer_types.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/api/v1alpha1/groupversion_info.go b/api/v1alpha1/groupversion_info.go index 04038ea..235a9d6 100644 --- a/api/v1alpha1/groupversion_info.go +++ b/api/v1alpha1/groupversion_info.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/api/v1alpha1/issuer_types.go b/api/v1alpha1/issuer_types.go index 4ed0944..159f3b7 100644 --- a/api/v1alpha1/issuer_types.go +++ b/api/v1alpha1/issuer_types.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/internal/controllers/certificaterequest_controller.go b/internal/controllers/certificaterequest_controller.go index ab4d84f..a544ca4 100644 --- a/internal/controllers/certificaterequest_controller.go +++ b/internal/controllers/certificaterequest_controller.go @@ -268,6 +268,8 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, nil } +// SetupWithManager registers the CertificateRequestReconciler with the controller manager. +// It configures controller-runtime to reconcile cert-manager CertificateRequests in the cluster. func (r *CertificateRequestReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&cmapi.CertificateRequest{}). diff --git a/internal/controllers/certificaterequest_controller_test.go b/internal/controllers/certificaterequest_controller_test.go index 075cb03..46cc9ba 100644 --- a/internal/controllers/certificaterequest_controller_test.go +++ b/internal/controllers/certificaterequest_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/internal/controllers/fake_configclient_test.go b/internal/controllers/fake_configclient_test.go index 6143d36..a654654 100644 --- a/internal/controllers/fake_configclient_test.go +++ b/internal/controllers/fake_configclient_test.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/internal/controllers/issuer_controller.go b/internal/controllers/issuer_controller.go index 10a0809..a2c8e53 100644 --- a/internal/controllers/issuer_controller.go +++ b/internal/controllers/issuer_controller.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -162,7 +162,8 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res return ctrl.Result{RequeueAfter: defaultHealthCheckInterval}, nil } -// SetupWithManager sets up the controller with the Manager. +// SetupWithManager registers the IssuerReconciler with the controller manager. +// It configures controller-runtime to reconcile Keyfactor Command Issuers/ClusterIssuers in the cluster. func (r *IssuerReconciler) SetupWithManager(mgr ctrl.Manager) error { issuerType, err := r.newIssuer() if err != nil { diff --git a/internal/controllers/issuer_controller_test.go b/internal/controllers/issuer_controller_test.go index 12c2c54..074b024 100644 --- a/internal/controllers/issuer_controller_test.go +++ b/internal/controllers/issuer_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 86c9493..f01a64c 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/internal/issuer/signer/signer.go b/internal/issuer/signer/signer.go index 187f55d..2e41281 100644 --- a/internal/issuer/signer/signer.go +++ b/internal/issuer/signer/signer.go @@ -66,6 +66,7 @@ type Signer interface { Sign(context.Context, []byte, K8sMetadata) ([]byte, []byte, error) } +// CommandHealthCheckerFromIssuerAndSecretData creates a new HealthChecker instance using the provided issuer spec and secret data func CommandHealthCheckerFromIssuerAndSecretData(ctx context.Context, spec *commandissuer.IssuerSpec, authSecretData map[string][]byte, caSecretData map[string][]byte) (HealthChecker, error) { signer := commandSigner{} @@ -79,10 +80,13 @@ func CommandHealthCheckerFromIssuerAndSecretData(ctx context.Context, spec *comm return &signer, nil } +// CommandSignerFromIssuerAndSecretData is a wrapper for commandSignerFromIssuerAndSecretData that returns a Signer interface +// given the provided issuer spec and secret data func CommandSignerFromIssuerAndSecretData(ctx context.Context, spec *commandissuer.IssuerSpec, annotations map[string]string, authSecretData map[string][]byte, caSecretData map[string][]byte) (Signer, error) { return commandSignerFromIssuerAndSecretData(ctx, spec, annotations, authSecretData, caSecretData) } +// commandSignerFromIssuerAndSecretData creates a new Signer instance using the provided issuer spec and secret data func commandSignerFromIssuerAndSecretData(ctx context.Context, spec *commandissuer.IssuerSpec, annotations map[string]string, authSecretData map[string][]byte, caSecretData map[string][]byte) (*commandSigner, error) { k8sLog := log.FromContext(ctx) @@ -132,6 +136,7 @@ func commandSignerFromIssuerAndSecretData(ctx context.Context, spec *commandissu return &signer, nil } +// extractMetadataFromAnnotations extracts metadata from the provided annotations func extractMetadataFromAnnotations(annotations map[string]string) map[string]interface{} { metadata := make(map[string]interface{}) @@ -144,6 +149,7 @@ func extractMetadataFromAnnotations(annotations map[string]string) map[string]in return metadata } +// Check checks the health of the signer by verifying that the "POST /Enrollment/CSR" endpoint exists func (s *commandSigner) Check() error { endpoints, _, err := s.client.StatusApi.StatusGetEndpoints(context.Background()).Execute() if err != nil { @@ -169,6 +175,7 @@ func (s *commandSigner) Check() error { return errors.New("missing \"POST /Enrollment/CSR\" endpoint") } +// Sign signs the provided CSR using the Keyfactor Command API func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMetadata) ([]byte, []byte, error) { k8sLog := log.FromContext(ctx) @@ -255,6 +262,8 @@ func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMe return compileCertificatesToPemBytes(certAndChain) } +// getCertificatesFromCertificateInformation takes a keyfactor.ModelsPkcs10CertificateResponse object and +// returns a slice of x509 certificates func getCertificatesFromCertificateInformation(commandResp *keyfactor.ModelsPkcs10CertificateResponse) ([]*x509.Certificate, error) { var certBytes []byte @@ -314,6 +323,7 @@ const ( CommandMetaCertificateSigningRequestNamespace = "Certificate-Signing-Request-Namespace" ) +// createCommandClientFromSecretData creates a new Keyfactor Command client using the provided issuer spec and secret data func createCommandClientFromSecretData(ctx context.Context, spec *commandissuer.IssuerSpec, authSecretData map[string][]byte, caSecretData map[string][]byte) (*keyfactor.APIClient, error) { k8sLogger := log.FromContext(ctx) @@ -383,6 +393,7 @@ func createCommandClientFromSecretData(ctx context.Context, spec *commandissuer. return client, nil } +// decodePEMBytes takes a byte array containing PEM encoded data and returns a slice of PEM blocks and a private key PEM block func decodePEMBytes(buf []byte) ([]*pem.Block, *pem.Block) { var privKey *pem.Block var certificates []*pem.Block @@ -400,6 +411,7 @@ func decodePEMBytes(buf []byte) ([]*pem.Block, *pem.Block) { return certificates, privKey } +// parseCSR takes a byte array containing a PEM encoded CSR and returns a x509.CertificateRequest object func parseCSR(pemBytes []byte) (*x509.CertificateRequest, error) { // extract PEM from request object block, _ := pem.Decode(pemBytes) @@ -409,6 +421,7 @@ func parseCSR(pemBytes []byte) (*x509.CertificateRequest, error) { return x509.ParseCertificateRequest(block.Bytes) } +// generateRandomString generates a random string of the specified length func generateRandomString(length int) string { rand.Seed(time.Now().UnixNano()) letters := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") @@ -419,6 +432,7 @@ func generateRandomString(length int) string { return string(b) } +// ptr returns a pointer to the provided value func ptr[T any](v T) *T { return &v } diff --git a/internal/issuer/signer/signer_test.go b/internal/issuer/signer/signer_test.go index 375ec9a..4ff4869 100644 --- a/internal/issuer/signer/signer_test.go +++ b/internal/issuer/signer/signer_test.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/internal/issuer/util/configclient.go b/internal/issuer/util/configclient.go index 990e9a1..db86470 100644 --- a/internal/issuer/util/configclient.go +++ b/internal/issuer/util/configclient.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -28,6 +28,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" ) +// ConfigClient is an interface for a K8s REST client. type ConfigClient interface { SetContext(ctx context.Context) GetConfigMap(name types.NamespacedName, out *corev1.ConfigMap) error @@ -43,6 +44,7 @@ type configClient struct { verifyAccessFunc func(apiResource string, resource types.NamespacedName) error } +// NewConfigClient creates a new K8s REST client using the configuration from the controller-runtime. func NewConfigClient(ctx context.Context) (ConfigClient, error) { config := ctrl.GetConfigOrDie() @@ -64,11 +66,15 @@ func NewConfigClient(ctx context.Context) (ConfigClient, error) { return client, nil } +// SetContext sets the context for the client. func (c *configClient) SetContext(ctx context.Context) { c.ctx = ctx c.logger = klog.FromContext(ctx) } +// verifyAccessToResource verifies that the client has access to a given resource in a given namespace +// by creating a SelfSubjectAccessReview. This is done to avoid errors when the client does not have +// access to the resource. func (c *configClient) verifyAccessToResource(apiResource string, resource types.NamespacedName) error { verbs := []string{"get", "list", "watch"} @@ -101,6 +107,7 @@ func (c *configClient) verifyAccessToResource(apiResource string, resource types return nil } +// GetConfigMap gets the configmap with the given name and namespace and copies it into the out parameter. func (c *configClient) GetConfigMap(name types.NamespacedName, out *corev1.ConfigMap) error { if c == nil { return fmt.Errorf("config client is nil") @@ -108,6 +115,8 @@ func (c *configClient) GetConfigMap(name types.NamespacedName, out *corev1.Confi // Check if the client has access to the configmap resource if _, ok := c.accessCache[name.String()]; !ok { + // If this is the first time the client is accessing the resource and it does have + // permission, add it to the access cache so that it does not need to be checked again. err := c.verifyAccessFunc("configmaps", name) if err != nil { return err @@ -126,6 +135,7 @@ func (c *configClient) GetConfigMap(name types.NamespacedName, out *corev1.Confi return nil } +// GetSecret gets the secret with the given name and namespace and copies it into the out parameter. func (c *configClient) GetSecret(name types.NamespacedName, out *corev1.Secret) error { if c == nil { return fmt.Errorf("config client is nil") @@ -133,6 +143,8 @@ func (c *configClient) GetSecret(name types.NamespacedName, out *corev1.Secret) // Check if the client has access to the secret resource if _, ok := c.accessCache[name.String()]; !ok { + // If this is the first time the client is accessing the resource and it does have + // permission, add it to the access cache so that it does not need to be checked again. err := c.verifyAccessFunc("secrets", name) if err != nil { return err diff --git a/internal/issuer/util/configclient_test.go b/internal/issuer/util/configclient_test.go index e57f922..5c30aad 100644 --- a/internal/issuer/util/configclient_test.go +++ b/internal/issuer/util/configclient_test.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/internal/issuer/util/util.go b/internal/issuer/util/util.go index f7477cc..4d513c2 100644 --- a/internal/issuer/util/util.go +++ b/internal/issuer/util/util.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -29,6 +29,7 @@ import ( const inClusterNamespacePath = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" +// GetSpecAndStatus is a helper function that returns the Spec and Status of an Issuer object. func GetSpecAndStatus(issuer client.Object) (*commandissuer.IssuerSpec, *commandissuer.IssuerStatus, error) { switch t := issuer.(type) { case *commandissuer.Issuer: @@ -40,6 +41,7 @@ func GetSpecAndStatus(issuer client.Object) (*commandissuer.IssuerSpec, *command } } +// SetReadyCondition is a helper function that sets the Ready condition on an IssuerStatus. func SetReadyCondition(status *commandissuer.IssuerStatus, conditionStatus commandissuer.ConditionStatus, reason, message string) { ready := GetReadyCondition(status) if ready == nil { @@ -64,6 +66,7 @@ func SetReadyCondition(status *commandissuer.IssuerStatus, conditionStatus comma } } +// GetReadyCondition is a helper function that returns the Ready condition from an IssuerStatus. func GetReadyCondition(status *commandissuer.IssuerStatus) *commandissuer.IssuerCondition { for _, c := range status.Conditions { if c.Type == commandissuer.IssuerConditionReady { @@ -73,6 +76,7 @@ func GetReadyCondition(status *commandissuer.IssuerStatus) *commandissuer.Issuer return nil } +// IsReady is a helper function that returns true if the Ready condition is set to True. func IsReady(status *commandissuer.IssuerStatus) bool { if c := GetReadyCondition(status); c != nil { return c.Status == commandissuer.ConditionTrue diff --git a/main.go b/main.go index 88fda9c..f51db11 100644 --- a/main.go +++ b/main.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Keyfactor Command Authors. +Copyright © 2023 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From 1d1fb258461920b950e4d40e77db1e84dfe0284f Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Thu, 14 Dec 2023 11:47:48 -0700 Subject: [PATCH 9/9] chore(comments): Write function comments and update license header --- internal/controllers/certificaterequest_controller.go | 2 ++ internal/controllers/issuer_controller.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/internal/controllers/certificaterequest_controller.go b/internal/controllers/certificaterequest_controller.go index a544ca4..b106a26 100644 --- a/internal/controllers/certificaterequest_controller.go +++ b/internal/controllers/certificaterequest_controller.go @@ -60,6 +60,8 @@ type CertificateRequestReconciler struct { // +kubebuilder:rbac:groups=cert-manager.io,resources=certificaterequests/status,verbs=get;update;patch // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch +// Reconcile attempts to sign a CertificateRequest given the configuration provided and a configured +// Command signer instance. func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) { log := ctrl.LoggerFrom(ctx) diff --git a/internal/controllers/issuer_controller.go b/internal/controllers/issuer_controller.go index a2c8e53..9f96af0 100644 --- a/internal/controllers/issuer_controller.go +++ b/internal/controllers/issuer_controller.go @@ -60,6 +60,7 @@ type IssuerReconciler struct { //+kubebuilder:rbac:groups=command-issuer.keyfactor.com,resources=issuers/status;clusterissuers/status,verbs=get;update;patch //+kubebuilder:rbac:groups=command-issuer.keyfactor.com,resources=issuers/finalizers,verbs=update +// newIssuer returns a new Issuer or ClusterIssuer object func (r *IssuerReconciler) newIssuer() (client.Object, error) { issuerGVK := commandissuer.GroupVersion.WithKind(r.Kind) ro, err := r.Scheme.New(issuerGVK) @@ -69,6 +70,7 @@ func (r *IssuerReconciler) newIssuer() (client.Object, error) { return ro.(client.Object), nil } +// Reconcile reconciles and updates the status of an Issuer or ClusterIssuer object func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) { log := ctrl.LoggerFrom(ctx)