From 91c79c4defa1c12c81ac9a8dd606d663aa3a8ef9 Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Fri, 24 Oct 2025 11:15:23 -0400 Subject: [PATCH 01/20] chore: update e2e tests to have better handling of testing cert -> issuance flow. Signed-off-by: Matthew H. Irby --- e2e/.env.example | 4 +- e2e/README.md | 46 +++++++- e2e/run_tests.sh | 274 +++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 302 insertions(+), 22 deletions(-) diff --git a/e2e/.env.example b/e2e/.env.example index 874e5fa..9dea707 100644 --- a/e2e/.env.example +++ b/e2e/.env.example @@ -7,4 +7,6 @@ export CERTIFICATE_AUTHORITY_LOGICAL_NAME="Sub-CA" export OAUTH_TOKEN_URL="https://example.com/oauth2/token" export OAUTH_CLIENT_ID="changeme" -export OAUTH_CLIENT_SECRET='changeme' \ No newline at end of file +export OAUTH_CLIENT_SECRET='changeme' +export OAUTH_SCOPES='optional' # remove if not needed +export OAUTH_AUDIENCE='optional' # remove if not needed \ No newline at end of file diff --git a/e2e/README.md b/e2e/README.md index f4fbe62..cc4ebc6 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -1,7 +1,49 @@ # End-to-End Test Suite -This is a test suite intended to make it easy to run tests on the command-cert-manager-issuer project. This suite can test the local changes of the command issuer, and it is able to test existing Docker images. +This is a test suite intended to make it easy to run end-to-end tests on the command-cert-manager-issuer project. This suite can test the local changes of the Command issuer, and it is able to test existing Docker images. + +The test suite does the following: +- Deploys command-cert-manager-issuer to a Kubernetes cluster with the desired version +- Creates an issuer (Issuer and ClusterIssuer) +- Creates a Certificate custom resource +- Waits for cert-manager to create a CertificateRequest, then signs the request +- Waits for the issuer to handle the CertificateRequest +- Verifies the CertificateRequest has been successfully processed and an issuer secret is created with the related certificate information. This is currently configured as a Bash script, so it is necessary to run this on a UNIX-compatible machine. -Instructions on how to run the e2e test suite are within the [run_tests.sh](./run_tests.sh) file. \ No newline at end of file +Instructions on how to run the e2e test suite are within the [run_tests.sh](./run_tests.sh) file. + +## Requirements +- An available Command instance is running and configured as described in the [root README](../README.md#configuring-command) + - OAuth is used to communicate with Command +- Docker (>= 28.2.2) +- Minikube (>= v1.35.0) +- kubectl (>= v1.32.2) +- helm (>= v3.17.1) +- cmctl (>= v2.1.1) + +## Configuring the environment variables +command-cert-manager-issuer interacts with an external Command instance. An environment variable file `.env` can be used to store the environment variables to be used to talk to the Command instance. + +A `.env.example` file is available as a template for your environment variables. + +```bash +# copy .env.example to .env +cp .env.example .env +``` + +Modify the fields as needed. + +## Running the script + +```bash +# enable the script to be executed +chmod +x ./run_test.sh + +# load the environment variables +source .env + +# run the end-to-end tests +./run_tests.sh +``` \ No newline at end of file diff --git a/e2e/run_tests.sh b/e2e/run_tests.sh index 84987d6..793774a 100755 --- a/e2e/run_tests.sh +++ b/e2e/run_tests.sh @@ -26,15 +26,6 @@ ## ======================================================================= -## ======================= Requirements =================================== -# - Minikube running -# - Helm installed -# - Docker installed -# - kubectl installed -# - cmctl installed -# - cert-manager Helm chart available -## =========================================================================== - ## ======================= How to run =================================== # Enable the script to run: # > chmod +x run_tests.sh @@ -52,13 +43,12 @@ IMAGE_TAG="local" # Uncomment if you want to build the image locally FULL_IMAGE_NAME="${IMAGE_REPO}/${IMAGE_NAME}:${IMAGE_TAG}" HELM_CHART_NAME="command-cert-manager-issuer" -#HELM_CHART_VERSION="2.1.0" # Uncomment if you want to use a specific version from the Helm repository +#H ELM_CHART_VERSION="2.1.0" # Uncomment if you want to use a specific version from the Helm repository HELM_CHART_VERSION="local" # Uncomment if you want to use the local Helm chart IS_LOCAL_DEPLOYMENT=$([ "$IMAGE_TAG" = "local" ] && echo "true" || echo "false") IS_LOCAL_HELM=$([ "$HELM_CHART_VERSION" = "local" ] && echo "true" || echo "false") -# TODO: Handle both in the e2e tests ISSUER_TYPE="Issuer" CLUSTER_ISSUER_TYPE="ClusterIssuer" @@ -85,13 +75,17 @@ ISSUER_NAMESPACE="issuer-playground" SIGNER_SECRET_NAME="auth-secret" SIGNER_CA_SECRET_NAME="ca-secret" +CERTIFICATE_CRD_FQTN="certificates.cert-manager.io" CERTIFICATEREQUEST_CRD_FQTN="certificaterequests.cert-manager.io" - -CR_CR_NAME="req" +CR_C_NAME="command-cert" +CR_CR_NAME="command-cert-1" +CR_C_SECRET_NAME="$CR_C_NAME-tls" set -e # Exit on any error +# checks if environment variable is available in system. if it is not present but the variable is required +# an error is thrown validate_env_present() { local env_var=$1 local required=$2 @@ -106,6 +100,7 @@ validate_env_present() { fi } +# checks whether the following environment variables are provided. some environment variables are optional. check_env() { validate_env_present HOSTNAME true validate_env_present API_PATH true @@ -114,10 +109,13 @@ check_env() { validate_env_present OAUTH_TOKEN_URL true validate_env_present OAUTH_CLIENT_ID true validate_env_present OAUTH_CLIENT_SECRET true + validate_env_present OAUTH_AUDIENCE false + validate_env_present OAUTH_SCOPES false validate_env_present CERTIFICATE_AUTHORITY_HOSTNAME false } +# checks whether the provided kubernetes namespace exists ns_exists () { local ns=$1 if [ "$(kubectl get namespace -o json | jq --arg namespace "$ns" -e '.items[] | select(.metadata.name == $namespace) | .metadata.name')" ]; then @@ -126,6 +124,7 @@ ns_exists () { return 1 } +# checks whether the provided helm chart has been deployed to the cluster (namespaced) helm_exists () { local namespace=$1 local chart_name=$2 @@ -135,6 +134,7 @@ helm_exists () { return 1 } +# checks whether the provided custom resource can be found in the cluster (namespaced) cr_exists () { local fqtn=$1 local ns=$2 @@ -146,6 +146,7 @@ cr_exists () { return 1 } +# checks whether the provided secret name exists in the cluster (namespaced) secret_exists () { local ns=$1 local name=$2 @@ -156,6 +157,7 @@ secret_exists () { return 1 } +# installs cert-manager onto the Kubernetes cluster install_cert_manager() { echo "๐Ÿ“ฆ Installing cert-manager..." @@ -179,6 +181,7 @@ install_cert_manager() { echo "โœ… cert-manager installed successfully" } +# installs the issuer to the Kubernetes cluster install_cert_manager_issuer() { echo "๐Ÿ“ฆ Installing instance of $IMAGE_NAME with tag $IMAGE_TAG..." @@ -194,6 +197,12 @@ install_cert_manager_issuer() { VERSION_PARAM="" else + # Add command-issuer repository if not already added + if ! helm repo list | grep -q command-issuer; then + echo "Adding command-issuer Helm repository..." + helm repo add command-issuer https://keyfactor.github.io/command-cert-manager-issuer + fi + CHART_PATH="command-issuer/command-cert-manager-issuer" echo "Using Helm chart from repository for version ${HELM_CHART_VERSION}: $CHART_PATH..." VERSION_PARAM="--version ${HELM_CHART_VERSION}" @@ -205,6 +214,15 @@ install_cert_manager_issuer() { else IMAGE_REPO_PARAM="" fi + + + + # Only set the pull policy to Never if we are deploying locally + if [[ "$IS_LOCAL_DEPLOYMENT" == "true" ]]; then + PULL_POLICY_PARAM="--set image.pullPolicy=Never" + else + PULL_POLICY_PARAM="" + fi # Helm chart could be out of date for release candidates, so we will install from # the chart defined in the repository. @@ -214,12 +232,116 @@ install_cert_manager_issuer() { $IMAGE_REPO_PARAM \ --set "fullnameOverride=${IMAGE_NAME}" \ --set image.tag=${IMAGE_TAG} \ - --set image.pullPolicy=Never \ - --wait + $PULL_POLICY_PARAM \ + --wait \ + --timeout 30s echo "โœ… $IMAGE_NAME installed successfully" } +# performs a redeployment of the cert-manager. helpful for recycling TLS certificates that have expired. +deploy_cert_manager() { + # Restart all cert-manager components + kubectl rollout restart deployment/cert-manager -n ${CERT_MANAGER_NAMESPACE} + kubectl rollout restart deployment/cert-manager-webhook -n ${CERT_MANAGER_NAMESPACE} + kubectl rollout restart deployment/cert-manager-cainjector -n ${CERT_MANAGER_NAMESPACE} + + # Wait for them to be ready + kubectl rollout status deployment/cert-manager -n ${CERT_MANAGER_NAMESPACE} + kubectl rollout status deployment/cert-manager-webhook -n ${CERT_MANAGER_NAMESPACE} + kubectl rollout status deployment/cert-manager-cainjector -n ${CERT_MANAGER_NAMESPACE} +} + +# deploys the issuer to the Kubernetes cluster +deploy_cert_manager_issuer() { + # Find the deployment name (assuming it follows a pattern) + DEPLOYMENT_NAME=$(kubectl get deployments -n ${MANAGER_NAMESPACE} -o jsonpath='{.items[0].metadata.name}' 2>/dev/null || echo "$IMAGE_NAME") + + # Between runs, we want to make sure that the running issuer has the latest version of the code we want. + # Doing this patch and redeployment forces the container to restart with the latest desired version + if kubectl get deployment ${DEPLOYMENT_NAME} -n ${MANAGER_NAMESPACE} >/dev/null 2>&1; then + # Patch the deployment + kubectl patch deployment ${DEPLOYMENT_NAME} -n ${MANAGER_NAMESPACE} -p "{ + \"spec\": { + \"template\": { + \"spec\": { + \"containers\": [{ + \"name\": \"${IMAGE_NAME}\", + \"image\": \"${FULL_IMAGE_NAME}\", + \"imagePullPolicy\": \"Never\" + }] + } + } + } + }" + + # Rollout deployment changes and apply the patch + kubectl rollout restart deployment/${DEPLOYMENT_NAME} -n ${MANAGER_NAMESPACE} + kubectl rollout status deployment/${DEPLOYMENT_NAME} -n ${MANAGER_NAMESPACE} --timeout=300s + + + echo "โœ… Deployment patched and rolled out successfully" + else + echo "โš ๏ธ Deployment ${DEPLOYMENT_NAME} not found. The Helm chart might use a different naming convention." + echo "Available deployments in ${MANAGER_NAMESPACE}:" + kubectl get deployments -n ${MANAGER_NAMESPACE} + fi + + echo "" + echo "๐ŸŽ‰ Deployment complete!" + echo "" +} + +# check the expiration of the cert-manager TLS certificate +check_cert_manager_webhook_cert() { + local namespace=${1:-cert-manager} + local secret_name=${2:-cert-manager-webhook-ca} + + echo "๐Ÿ” Checking cert-manager webhook certificate..." + + # Check if secret exists + if ! kubectl get secret "$secret_name" -n "$namespace" >/dev/null 2>&1; then + echo "โŒ Secret $secret_name not found in namespace $namespace" + return 1 + fi + + # Get certificate data + local cert_data=$(kubectl get secret "$secret_name" -n "$namespace" -o jsonpath='{.data.tls\.crt}' 2>/dev/null) + + if [ -z "$cert_data" ]; then + echo "โŒ No certificate data found in secret" + return 1 + fi + + # Decode and check certificate + local cert_info=$(echo "$cert_data" | base64 -d | openssl x509 -noout -dates 2>/dev/null) + + if [ $? -ne 0 ]; then + echo "โŒ Failed to parse certificate" + return 1 + fi + + echo "๐Ÿ“‹ Certificate validity:" + echo "$cert_info" + + # Check if certificate is currently valid + if echo "$cert_data" | base64 -d | openssl x509 -noout -checkend 0 >/dev/null 2>&1; then + echo "โœ… Certificate is currently valid" + + # Check if expires within 7 days + if ! echo "$cert_data" | base64 -d | openssl x509 -noout -checkend 604800 >/dev/null 2>&1; then + echo "โš ๏ธ Certificate expires within 7 days" + return 2 # Warning status + fi + + return 0 # Valid + else + echo "โŒ Certificate is expired or not yet valid" + return 1 # Expired + fi +} + +# creates a new issuer custom resource create_issuer() { echo "๐Ÿ” Creating issuer resource..." @@ -260,6 +382,7 @@ EOF echo "โœ… Issuer resources created successfully" } +# creates a new cluster issuer custom resource create_cluster_issuer() { echo "๐Ÿ” Creating cluster issuer resource..." @@ -300,6 +423,7 @@ EOF echo "โœ… Issuer resources created successfully" } +# deletes Issuer and ClusterIssuer custom resources from the Kubernetes cluster delete_issuers() { echo "๐Ÿ—‘๏ธ Deleting issuer resources..." @@ -323,6 +447,59 @@ delete_issuers() { echo "โœ… Issuer resources deleted successfully" } +# creates a Certificate custom resource. this is picked up by cert-manager and converted to a CertificateRequest. +create_certificate() { + local issuer_type=$1 + + echo "Generating a certificate object for issuer type: $issuer_type" + + kubectl -n "$ISSUER_NAMESPACE" apply -f - < Date: Fri, 24 Oct 2025 11:33:23 -0400 Subject: [PATCH 02/20] chore: regenerate crd's with 'make generate manifests' command Signed-off-by: Matthew H. Irby --- ...d-issuer.keyfactor.com_clusterissuers.yaml | 48 +++++++++---------- .../command-issuer.keyfactor.com_issuers.yaml | 48 +++++++++---------- 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml index 0d7feef..ae0aebf 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml @@ -68,31 +68,51 @@ spec: CertificateAuthorityLogicalName is the logical name of the certificate authority to use E.g. "Keyfactor Root CA" or "Intermediate CA" type: string + certificateTemplate: + description: |- + Deprecated. CertificateTemplate is the name of the certificate template to use. If using Keyfactor Command 25.1 or later, use EnrollmentPatternName or EnrollmentPatternId instead. + If both enrollment pattern and certificate template are specified, enrollment pattern will take precedence. + Enrollment will fail if the specified template is not compatible with the enrollment pattern. + Refer to the Keyfactor Command documentation for more information. + type: string + commandSecretName: + description: |- + A reference to a K8s kubernetes.io/basic-auth Secret containing basic auth + credentials for the Command instance configured in Hostname. The secret must + be in the same namespace as the referent. If the + referent is a ClusterIssuer, the reference instead refers to the resource + with the given name in the configured 'cluster resource namespace', which + is set as a flag on the controller component (and defaults to the + namespace that the controller runs in). + type: string enrollmentPatternId: description: |- EnrollmentPatternId is the ID of the enrollment pattern to use. Supported in Keyfactor Command 25.1 and later. If both enrollment pattern and certificate template are specified, enrollment pattern will take precedence. - If both enrollmentPatternId and enrollmentPatternName are specified, enrollmentPatternId will take precedence. + If EnrollmentPatternId and EnrollmentPatternName are both specified, EnrollmentPatternId will take precedence. Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. - type: integer format: int32 + type: integer enrollmentPatternName: description: |- EnrollmentPatternName is the name of the enrollment pattern to use. Supported in Keyfactor Command 25.1 and later. If both enrollment pattern and certificate template are specified, enrollment pattern will take precedence. - If both enrollmentPatternId and enrollmentPatternName are specified, enrollmentPatternId will take precedence. + If EnrollmentPatternId and EnrollmentPatternName are both specified, EnrollmentPatternId will take precedence. Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. type: string + hostname: + description: Hostname is the hostname of a Keyfactor Command instance. + type: string ownerRoleId: description: |- OwnerRoleId is the ID of the security role assigned as the certificate owner. The specified security role must be assigned to the authorized identity context. If OwnerRoleId and OwnerRoleName are both specified, OwnerRoleId will take precedence. This field is required if the enrollment pattern, certificate template, or system-wide settings has been configured as Required. - type: integer format: int32 + type: integer ownerRoleName: description: |- OwnerRoleName is the name of the security role assigned as the certificate owner. This name must match the existing name of the security role. @@ -100,26 +120,6 @@ spec: If OwnerRoleId and OwnerRoleName are both specified, OwnerRoleId will take precedence. This field is required if the enrollment pattern, certificate template, or system-wide settings has been configured as Required. type: string - certificateTemplate: - description: |- - CertificateTemplate is the name of the certificate template to use. Deprecated in favor of EnrollmentPattern as of Keyfactor Command 25.1. - If both enrollment pattern and certificate template are specified, enrollment pattern will take precedence. - Enrollment will fail if the specified template is not compatible with the enrollment pattern. - Refer to the Keyfactor Command documentation for more information. - type: string - commandSecretName: - description: |- - A reference to a K8s kubernetes.io/basic-auth Secret containing basic auth - credentials for the Command instance configured in Hostname. The secret must - be in the same namespace as the referent. If the - referent is a ClusterIssuer, the reference instead refers to the resource - with the given name in the configured 'cluster resource namespace', which - is set as a flag on the controller component (and defaults to the - namespace that the controller runs in). - type: string - hostname: - description: Hostname is the hostname of a Keyfactor Command instance. - type: string scopes: description: |- A list of comma separated scopes used when requesting a Bearer token from an ambient token provider implied diff --git a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml index a4034ce..33ab160 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml @@ -68,31 +68,51 @@ spec: CertificateAuthorityLogicalName is the logical name of the certificate authority to use E.g. "Keyfactor Root CA" or "Intermediate CA" type: string + certificateTemplate: + description: |- + Deprecated. CertificateTemplate is the name of the certificate template to use. If using Keyfactor Command 25.1 or later, use EnrollmentPatternName or EnrollmentPatternId instead. + If both enrollment pattern and certificate template are specified, enrollment pattern will take precedence. + Enrollment will fail if the specified template is not compatible with the enrollment pattern. + Refer to the Keyfactor Command documentation for more information. + type: string + commandSecretName: + description: |- + A reference to a K8s kubernetes.io/basic-auth Secret containing basic auth + credentials for the Command instance configured in Hostname. The secret must + be in the same namespace as the referent. If the + referent is a ClusterIssuer, the reference instead refers to the resource + with the given name in the configured 'cluster resource namespace', which + is set as a flag on the controller component (and defaults to the + namespace that the controller runs in). + type: string enrollmentPatternId: description: |- EnrollmentPatternId is the ID of the enrollment pattern to use. Supported in Keyfactor Command 25.1 and later. If both enrollment pattern and certificate template are specified, enrollment pattern will take precedence. - If both enrollmentPatternId and enrollmentPatternName are specified, enrollmentPatternId will take precedence. + If EnrollmentPatternId and EnrollmentPatternName are both specified, EnrollmentPatternId will take precedence. Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. - type: integer format: int32 + type: integer enrollmentPatternName: description: |- EnrollmentPatternName is the name of the enrollment pattern to use. Supported in Keyfactor Command 25.1 and later. If both enrollment pattern and certificate template are specified, enrollment pattern will take precedence. - If both enrollmentPatternId and enrollmentPatternName are specified, enrollmentPatternId will take precedence. + If EnrollmentPatternId and EnrollmentPatternName are both specified, EnrollmentPatternId will take precedence. Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. type: string + hostname: + description: Hostname is the hostname of a Keyfactor Command instance. + type: string ownerRoleId: description: |- OwnerRoleId is the ID of the security role assigned as the certificate owner. The specified security role must be assigned to the authorized identity context. If OwnerRoleId and OwnerRoleName are both specified, OwnerRoleId will take precedence. This field is required if the enrollment pattern, certificate template, or system-wide settings has been configured as Required. - type: integer format: int32 + type: integer ownerRoleName: description: |- OwnerRoleName is the name of the security role assigned as the certificate owner. This name must match the existing name of the security role. @@ -100,26 +120,6 @@ spec: If OwnerRoleId and OwnerRoleName are both specified, OwnerRoleId will take precedence. This field is required if the enrollment pattern, certificate template, or system-wide settings has been configured as Required. type: string - certificateTemplate: - description: |- - CertificateTemplate is the name of the certificate template to use. Deprecated in favor of EnrollmentPattern as of Keyfactor Command 25.1. - If both enrollment pattern and certificate template are specified, enrollment pattern will take precedence. - Enrollment will fail if the specified template is not compatible with the enrollment pattern. - Refer to the Keyfactor Command documentation for more information. - type: string - commandSecretName: - description: |- - A reference to a K8s kubernetes.io/basic-auth Secret containing basic auth - credentials for the Command instance configured in Hostname. The secret must - be in the same namespace as the referent. If the - referent is a ClusterIssuer, the reference instead refers to the resource - with the given name in the configured 'cluster resource namespace', which - is set as a flag on the controller component (and defaults to the - namespace that the controller runs in). - type: string - hostname: - description: Hostname is the hostname of a Keyfactor Command instance. - type: string scopes: description: |- A list of comma separated scopes used when requesting a Bearer token from an ambient token provider implied From b9b63250976fbd5f1e71d12a11566a0893f35476 Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Fri, 24 Oct 2025 13:11:09 -0400 Subject: [PATCH 03/20] feat: add health check interval to issuer spec Signed-off-by: Matthew H. Irby --- api/v1alpha1/issuer_types.go | 4 ++++ api/v1alpha1/zz_generated.deepcopy.go | 9 +++++++-- .../command-issuer.keyfactor.com_clusterissuers.yaml | 5 +++++ .../crd/bases/command-issuer.keyfactor.com_issuers.yaml | 5 +++++ .../templates/crds/clusterissuers.yaml | 5 +++++ .../templates/crds/issuers.yaml | 5 +++++ 6 files changed, 31 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/issuer_types.go b/api/v1alpha1/issuer_types.go index ac0cdc7..af39276 100644 --- a/api/v1alpha1/issuer_types.go +++ b/api/v1alpha1/issuer_types.go @@ -46,6 +46,10 @@ type IssuerSpec struct { // +kubebuilder:default:=KeyfactorAPI APIPath string `json:"apiPath,omitempty"` + // The number of seconds between successful health checks. 60 seconds (1 minute) by default + // +kubebuilder:default:=60 + HealthCheckIntervalSeconds *int `json:"healthCheckIntervalSeconds,omitempty"` + // EnrollmentPatternId is the ID of the enrollment pattern to use. Supported in Keyfactor Command 25.1 and later. // If both enrollment pattern and certificate template are specified, enrollment pattern will take precedence. // If EnrollmentPatternId and EnrollmentPatternName are both specified, EnrollmentPatternId will take precedence. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 3eb08e1..9555ca1 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -29,7 +29,7 @@ func (in *ClusterIssuer) DeepCopyInto(out *ClusterIssuer) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) } @@ -88,7 +88,7 @@ func (in *Issuer) DeepCopyInto(out *Issuer) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) } @@ -164,6 +164,11 @@ func (in *IssuerList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IssuerSpec) DeepCopyInto(out *IssuerSpec) { *out = *in + if in.HealthCheckIntervalSeconds != nil { + in, out := &in.HealthCheckIntervalSeconds, &out.HealthCheckIntervalSeconds + *out = new(int) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IssuerSpec. diff --git a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml index ae0aebf..c6cee63 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml @@ -102,6 +102,11 @@ spec: Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. type: string + healthCheckIntervalSeconds: + default: 60 + description: The number of seconds between successful health checks. + 60 seconds (1 minute) by default + type: integer hostname: description: Hostname is the hostname of a Keyfactor Command instance. type: string diff --git a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml index 33ab160..b9cb24f 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml @@ -102,6 +102,11 @@ spec: Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. type: string + healthCheckIntervalSeconds: + default: 60 + description: The number of seconds between successful health checks. + 60 seconds (1 minute) by default + type: integer hostname: description: Hostname is the hostname of a Keyfactor Command instance. type: string diff --git a/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml b/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml index ec8b4a9..fd7f4e2 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml @@ -79,6 +79,11 @@ spec: Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. type: string + healthCheckIntervalSeconds: + default: 60 + description: The number of seconds between successful health checks. + 60 seconds (1 minute) by default + type: integer ownerRoleId: description: |- OwnerRoleId is the ID of the security role assigned as the certificate owner. diff --git a/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml b/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml index ea377ca..8185ca4 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml @@ -79,6 +79,11 @@ spec: Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. type: string + healthCheckIntervalSeconds: + default: 60 + description: The number of seconds between successful health checks. + 60 seconds (1 minute) by default + type: integer ownerRoleId: description: |- OwnerRoleId is the ID of the security role assigned as the certificate owner. From 743d8ff76bb2a856abe39fe21e3b2230719cba79 Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Fri, 24 Oct 2025 13:57:28 -0400 Subject: [PATCH 04/20] chore: update docs Signed-off-by: Matthew H. Irby --- api/v1alpha1/issuer_types.go | 2 +- .../crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml | 3 ++- config/crd/bases/command-issuer.keyfactor.com_issuers.yaml | 3 ++- .../templates/crds/clusterissuers.yaml | 3 ++- .../command-cert-manager-issuer/templates/crds/issuers.yaml | 3 ++- docsource/content.md | 3 +++ 6 files changed, 12 insertions(+), 5 deletions(-) diff --git a/api/v1alpha1/issuer_types.go b/api/v1alpha1/issuer_types.go index af39276..b96c9c8 100644 --- a/api/v1alpha1/issuer_types.go +++ b/api/v1alpha1/issuer_types.go @@ -46,7 +46,7 @@ type IssuerSpec struct { // +kubebuilder:default:=KeyfactorAPI APIPath string `json:"apiPath,omitempty"` - // The number of seconds between successful health checks. 60 seconds (1 minute) by default + // The number of seconds between successful health checks. 60 seconds (1 minute) by default. Setting to 0 will disable the health check. // +kubebuilder:default:=60 HealthCheckIntervalSeconds *int `json:"healthCheckIntervalSeconds,omitempty"` diff --git a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml index c6cee63..6e80de0 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml @@ -105,7 +105,8 @@ spec: healthCheckIntervalSeconds: default: 60 description: The number of seconds between successful health checks. - 60 seconds (1 minute) by default + 60 seconds (1 minute) by default. Setting to 0 will disable the + health check. type: integer hostname: description: Hostname is the hostname of a Keyfactor Command instance. diff --git a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml index b9cb24f..3bd45d3 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml @@ -105,7 +105,8 @@ spec: healthCheckIntervalSeconds: default: 60 description: The number of seconds between successful health checks. - 60 seconds (1 minute) by default + 60 seconds (1 minute) by default. Setting to 0 will disable the + health check. type: integer hostname: description: Hostname is the hostname of a Keyfactor Command instance. diff --git a/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml b/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml index fd7f4e2..a0f8e5b 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml @@ -82,7 +82,8 @@ spec: healthCheckIntervalSeconds: default: 60 description: The number of seconds between successful health checks. - 60 seconds (1 minute) by default + 60 seconds (1 minute) by default. Setting to 0 will disable the + health check. type: integer ownerRoleId: description: |- diff --git a/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml b/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml index 8185ca4..df6b81b 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml @@ -82,7 +82,8 @@ spec: healthCheckIntervalSeconds: default: 60 description: The number of seconds between successful health checks. - 60 seconds (1 minute) by default + 60 seconds (1 minute) by default. Setting to 0 will disable the + health check. type: integer ownerRoleId: description: |- diff --git a/docsource/content.md b/docsource/content.md index 898600a..d6fa661 100644 --- a/docsource/content.md +++ b/docsource/content.md @@ -219,6 +219,7 @@ For example, ClusterIssuer resources can be used to issue certificates for resou | ownerRoleName | The name of the security role assigned as the certificate owner. The security role must be assigned to the identity context of the issuer. If `ownerRoleId` and `ownerRoleName` are both specified, `ownerRoleId` will take precedence. This field is **required** if the enrollment pattern, certificate template, or system-wide setting requires it. | | scopes | (Optional) Required if using ambient credentials with Azure AKS. If using ambient credentials, these scopes will be put on the access token generated by the ambient credentials' token provider, if applicable. | | audience | (Optional) If using ambient credentials, this audience will be put on the access token generated by the ambient credentials' token provider, if applicable. Google's ambient credential token provider generates an OIDC ID Token. If this value is not provided, it will default to `command`. | + | healthCheckIntervalSeconds | (Optional) Defines the health check interval, in seconds, for a healthy issuer. If ommitted, defaults to 60 seconds. If set to 0, it will disable the health check. If there is a failure when running the health check, it will retry in 10 seconds with an exponential backoff strategy. Value must not be negative. | > If a different combination of hostname/certificate authority/certificate template is required, a new Issuer or ClusterIssuer resource must be created. Each resource instantiation represents a single configuration. @@ -250,6 +251,7 @@ For example, ClusterIssuer resources can be used to issue certificates for resou # ownerRoleName: "$OWNER_ROLE_NAME" # Uncomment if required # scopes: "openid email https://example.com/.default" # Uncomment if required # audience: "https://your-command-url.com" # Uncomment if desired + # healthCheckIntervalSeconds: 60 # Uncomment if desired. Setting to 0 disables health check. EOF kubectl -n default apply -f issuer.yaml @@ -280,6 +282,7 @@ For example, ClusterIssuer resources can be used to issue certificates for resou # ownerRoleName: "$OWNER_ROLE_NAME" # Uncomment if required # scopes: "openid email https://example.com/.default" # Uncomment if required # audience: "https://your-command-url.com" # Uncomment if desired + # healthCheckIntervalSeconds: 60 # Uncomment if desired. Setting to 0 disables health check. EOF kubectl apply -f clusterissuer.yaml From 9b4c278267ce75e3b9577cdd7441c5de424a9156 Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Fri, 24 Oct 2025 14:10:12 -0400 Subject: [PATCH 05/20] feat: implement health check logic Signed-off-by: Matthew H. Irby --- CHANGELOG.md | 4 + internal/controller/issuer_controller.go | 39 +++++- internal/controller/issuer_controller_test.go | 131 ++++++++++++++++-- 3 files changed, 162 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c5a6df..482dda4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# v2.3.2 +## Features +- Add a `healthCheckIntervalSeconds` specification to Issuer / ClusterIssuer resources, allowing flexibility in the health check interval. + # v2.3.1 ## Fixes - Add a manual dispatch of Helm chart release. diff --git a/internal/controller/issuer_controller.go b/internal/controller/issuer_controller.go index 5c44ec7..a986130 100644 --- a/internal/controller/issuer_controller.go +++ b/internal/controller/issuer_controller.go @@ -25,6 +25,7 @@ import ( commandissuer "github.com/Keyfactor/command-cert-manager-issuer/api/v1alpha1" "github.com/Keyfactor/command-cert-manager-issuer/internal/command" + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -86,8 +87,6 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res return ctrl.Result{}, nil } - log.Info(fmt.Sprintf("Starting %s reconciliation run", issuer.GetObjectKind().GroupVersionKind().Kind)) - // Always attempt to update the Ready condition defer func() { if err != nil { @@ -99,6 +98,17 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res } }() + healthCheckInterval, err := getHealthCheckInterval(log, issuer) + if err != nil { + log.Error(err, "en error occurred while getting the health check interval") + issuer.GetStatus().SetCondition(ctx, commandissuer.IssuerConditionReady, commandissuer.ConditionFalse, issuerReadyConditionReason, err.Error()) + issuer.GetStatus().SetCondition(ctx, commandissuer.IssuerConditionSupportsMetadata, commandissuer.ConditionUnknown, "", "") + return ctrl.Result{}, nil + } + + log.Info(fmt.Sprintf("Starting %s reconciliation run", issuer.GetObjectKind().GroupVersionKind().Kind)) + log.Info(fmt.Sprintf("Issuer %s has been configured with a health check interval of %d seconds", issuer.GetObjectKind().GroupVersionKind().Kind, int(healthCheckInterval/time.Second))) + var secretNamespace string switch { @@ -142,7 +152,30 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res issuer.GetStatus().SetCondition(ctx, commandissuer.IssuerConditionSupportsMetadata, commandissuer.ConditionFalse, "Metadata fields are not defined", "Connected Command platform doesn't have the Command Issuer metadata fields defined.") } - return ctrl.Result{RequeueAfter: defaultHealthCheckInterval}, nil + return ctrl.Result{RequeueAfter: healthCheckInterval}, nil +} + +func getHealthCheckInterval(log logr.Logger, issuer commandissuer.IssuerLike) (time.Duration, error) { + spec := issuer.GetSpec() + + if spec.HealthCheckIntervalSeconds == nil { + log.Info(fmt.Sprintf("health check spec value is nil, using default: %d", int(defaultHealthCheckInterval/time.Second))) + return defaultHealthCheckInterval, nil + } + + interval := *spec.HealthCheckIntervalSeconds + + // Health check interval should not be negative + if interval < 0 { + return 0, fmt.Errorf("interval %d is invalid, must be greater than or equal to 0", interval) + } + + // Issuer may be configured to ignore future health checks + if interval == 0 { + log.Info("health check interval is configured to be 0. this will disable future health checks for issuer.") + } + + return time.Duration(interval) * time.Second, nil } func commandConfigFromIssuer(ctx context.Context, c client.Client, issuer commandissuer.IssuerLike, secretNamespace string) (*command.Config, error) { diff --git a/internal/controller/issuer_controller_test.go b/internal/controller/issuer_controller_test.go index fe754c5..af952b8 100644 --- a/internal/controller/issuer_controller_test.go +++ b/internal/controller/issuer_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright ยฉ 2024 Keyfactor +Copyright ยฉ 2025 Keyfactor Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -20,7 +20,9 @@ import ( "context" "errors" "testing" + "time" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" commandissuerv1alpha1 "github.com/Keyfactor/command-cert-manager-issuer/api/v1alpha1" "github.com/Keyfactor/command-cert-manager-issuer/internal/command" logrtesting "github.com/go-logr/logr/testing" @@ -52,19 +54,13 @@ func (f *fakeHealthChecker) CommandSupportsMetadata() (bool, error) { var newFakeHealthCheckerBuilder = func(builderErr error, checkerErr error, supportsMetadata bool) func(context.Context, *command.Config) (command.HealthChecker, error) { return func(context.Context, *command.Config) (command.HealthChecker, error) { return &fakeHealthChecker{ - errCheck: checkerErr, + supportsMetadata: supportsMetadata, + errCheck: checkerErr, }, builderErr } } func TestIssuerReconcile(t *testing.T) { - // caCert, rootKey := issueTestCertificate(t, "Root-CA", nil, nil) - // caCertPem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: caCert.Raw}) - - // serverCert, _ := issueTestCertificate(t, "Server", caCert, rootKey) - // serverCertPem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: serverCert.Raw}) - // caChain := append(serverCertPem, caCertPem...) - type testCase struct { kind string name types.NamespacedName @@ -572,6 +568,123 @@ func TestIssuerReconcile(t *testing.T) { expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, }, + "success-custom-healthcheck-interval-issuer": { + kind: "Issuer", + name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, + objects: []client.Object{ + &commandissuerv1alpha1.Issuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "issuer1", + Namespace: "ns1", + }, + Spec: commandissuerv1alpha1.IssuerSpec{ + SecretName: "issuer1-credentials", + HealthCheckIntervalSeconds: to.Ptr(30), + }, + Status: commandissuerv1alpha1.IssuerStatus{ + Conditions: []commandissuerv1alpha1.IssuerCondition{ + { + Type: commandissuerv1alpha1.IssuerConditionReady, + Status: commandissuerv1alpha1.ConditionUnknown, + }, + }, + }, + }, + &corev1.Secret{ + Type: corev1.SecretTypeBasicAuth, + ObjectMeta: metav1.ObjectMeta{ + Name: "issuer1-credentials", + Namespace: "ns1", + }, + Data: map[string][]byte{ + corev1.BasicAuthUsernameKey: []byte("username"), + corev1.BasicAuthPasswordKey: []byte("password"), + }, + }, + }, + healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), + expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedResult: ctrl.Result{RequeueAfter: time.Duration(30) * time.Second}, + }, + "success-custom-healthcheck-interval-clusterissuer": { + kind: "ClusterIssuer", + name: types.NamespacedName{Name: "clusterissuer1"}, + objects: []client.Object{ + &commandissuerv1alpha1.ClusterIssuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1", + }, + Spec: commandissuerv1alpha1.IssuerSpec{ + SecretName: "clusterissuer1-credentials", + HealthCheckIntervalSeconds: to.Ptr(30), + }, + Status: commandissuerv1alpha1.IssuerStatus{ + Conditions: []commandissuerv1alpha1.IssuerCondition{ + { + Type: commandissuerv1alpha1.IssuerConditionReady, + Status: commandissuerv1alpha1.ConditionUnknown, + }, + }, + }, + }, + &corev1.Secret{ + Type: corev1.SecretTypeBasicAuth, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1-credentials", + Namespace: "kube-system", + }, + Data: map[string][]byte{ + corev1.BasicAuthUsernameKey: []byte("username"), + corev1.BasicAuthPasswordKey: []byte("password"), + }, + }, + }, + healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), + clusterResourceNamespace: "kube-system", + expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedResult: ctrl.Result{RequeueAfter: time.Duration(30) * time.Second}, + }, + "error-healthcheck-negative-value": { + kind: "Issuer", + name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, + objects: []client.Object{ + &commandissuerv1alpha1.Issuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "issuer1", + Namespace: "ns1", + }, + Spec: commandissuerv1alpha1.IssuerSpec{ + SecretName: "issuer1-credentials", + HealthCheckIntervalSeconds: to.Ptr(-30), + }, + Status: commandissuerv1alpha1.IssuerStatus{ + Conditions: []commandissuerv1alpha1.IssuerCondition{ + { + Type: commandissuerv1alpha1.IssuerConditionReady, + Status: commandissuerv1alpha1.ConditionUnknown, + }, + }, + }, + }, + &corev1.Secret{ + Type: corev1.SecretTypeBasicAuth, + ObjectMeta: metav1.ObjectMeta{ + Name: "issuer1-credentials", + Namespace: "ns1", + }, + Data: map[string][]byte{ + corev1.BasicAuthUsernameKey: []byte("username"), + corev1.BasicAuthPasswordKey: []byte("password"), + }, + }, + }, + healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false), + expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionUnknown, + expectedResult: ctrl.Result{}, + }, } scheme := runtime.NewScheme() From d9224174088456cd23f44b867d5e8da4ed88ddac Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Fri, 24 Oct 2025 14:10:30 -0400 Subject: [PATCH 06/20] feat(actions): Ensure that CRDs are not out of date Signed-off-by: Matthew H. Irby --- .github/workflows/keyfactor-bootstrap-workflow.yml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/keyfactor-bootstrap-workflow.yml b/.github/workflows/keyfactor-bootstrap-workflow.yml index f72b649..b4fff0e 100644 --- a/.github/workflows/keyfactor-bootstrap-workflow.yml +++ b/.github/workflows/keyfactor-bootstrap-workflow.yml @@ -12,7 +12,7 @@ on: jobs: build: - name: Build and Lint + name: Build and Check CRDs runs-on: ubuntu-latest timeout-minutes: 8 steps: @@ -23,11 +23,17 @@ jobs: cache: true - run: go mod download - run: go build -v ./cmd/main.go + - name: Regenerate CRDs + run: make generate manifests + - name: Check for CRD drift + run: | + git diff --compact-summary --exit-code || \ + (echo; echo "Unexpected difference in directories after code generation. Run 'make generate manifests' and commit."; exit 1) # - name: Run linters # uses: golangci/golangci-lint-action@08e2f20817b15149a52b5b3ebe7de50aff2ba8c5 # v3.4.0 # with: # version: latest - + test: name: Go Test needs: build From 7067c544768638a2170dafa898b43fcdaa66df4a Mon Sep 17 00:00:00 2001 From: Keyfactor Date: Fri, 24 Oct 2025 18:17:13 +0000 Subject: [PATCH 07/20] Update generated docs --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 2d3ce53..0b3beef 100644 --- a/README.md +++ b/README.md @@ -251,6 +251,7 @@ For example, ClusterIssuer resources can be used to issue certificates for resou | ownerRoleName | The name of the security role assigned as the certificate owner. The security role must be assigned to the identity context of the issuer. If `ownerRoleId` and `ownerRoleName` are both specified, `ownerRoleId` will take precedence. This field is **required** if the enrollment pattern, certificate template, or system-wide setting requires it. | | scopes | (Optional) Required if using ambient credentials with Azure AKS. If using ambient credentials, these scopes will be put on the access token generated by the ambient credentials' token provider, if applicable. | | audience | (Optional) If using ambient credentials, this audience will be put on the access token generated by the ambient credentials' token provider, if applicable. Google's ambient credential token provider generates an OIDC ID Token. If this value is not provided, it will default to `command`. | + | healthCheckIntervalSeconds | (Optional) Defines the health check interval, in seconds, for a healthy issuer. If ommitted, defaults to 60 seconds. If set to 0, it will disable the health check. If there is a failure when running the health check, it will retry in 10 seconds with an exponential backoff strategy. Value must not be negative. | > If a different combination of hostname/certificate authority/certificate template is required, a new Issuer or ClusterIssuer resource must be created. Each resource instantiation represents a single configuration. @@ -282,6 +283,7 @@ For example, ClusterIssuer resources can be used to issue certificates for resou # ownerRoleName: "$OWNER_ROLE_NAME" # Uncomment if required # scopes: "openid email https://example.com/.default" # Uncomment if required # audience: "https://your-command-url.com" # Uncomment if desired + # healthCheckIntervalSeconds: 60 # Uncomment if desired. Setting to 0 disables health check. EOF kubectl -n default apply -f issuer.yaml @@ -312,6 +314,7 @@ For example, ClusterIssuer resources can be used to issue certificates for resou # ownerRoleName: "$OWNER_ROLE_NAME" # Uncomment if required # scopes: "openid email https://example.com/.default" # Uncomment if required # audience: "https://your-command-url.com" # Uncomment if desired + # healthCheckIntervalSeconds: 60 # Uncomment if desired. Setting to 0 disables health check. EOF kubectl apply -f clusterissuer.yaml From 2f9f9b93001563964b88c6d372dd6d35a10e05a6 Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Fri, 24 Oct 2025 14:21:36 -0400 Subject: [PATCH 08/20] chore(docs): update changelog version number Signed-off-by: Matthew H. Irby --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 482dda4..1887129 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -# v2.3.2 +# v2.3.3 ## Features - Add a `healthCheckIntervalSeconds` specification to Issuer / ClusterIssuer resources, allowing flexibility in the health check interval. From 52443ce249f0db27150e74f1dc7b33fc18459f52 Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Mon, 27 Oct 2025 12:16:14 -0400 Subject: [PATCH 09/20] chore: Convert to healthcheck block instead of healthCheckIntervalSeconds Signed-off-by: Matthew H. Irby --- CHANGELOG.md | 2 +- api/v1alpha1/issuer_types.go | 16 +- api/v1alpha1/zz_generated.deepcopy.go | 29 ++- ...d-issuer.keyfactor.com_clusterissuers.yaml | 22 +- .../command-issuer.keyfactor.com_issuers.yaml | 22 +- .../templates/crds/clusterissuers.yaml | 22 +- .../templates/crds/issuers.yaml | 22 +- docsource/content.md | 12 +- e2e/run_tests.sh | 4 +- internal/controller/issuer_controller.go | 27 +- internal/controller/issuer_controller_test.go | 231 +++++++++++++++++- 11 files changed, 354 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1887129..9539c3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # v2.3.3 ## Features -- Add a `healthCheckIntervalSeconds` specification to Issuer / ClusterIssuer resources, allowing flexibility in the health check interval. +- Add a `healthcheck` specification to Issuer / ClusterIssuer resources, allowing flexibility in the health check interval. # v2.3.1 ## Fixes diff --git a/api/v1alpha1/issuer_types.go b/api/v1alpha1/issuer_types.go index b96c9c8..f9f3329 100644 --- a/api/v1alpha1/issuer_types.go +++ b/api/v1alpha1/issuer_types.go @@ -46,9 +46,10 @@ type IssuerSpec struct { // +kubebuilder:default:=KeyfactorAPI APIPath string `json:"apiPath,omitempty"` - // The number of seconds between successful health checks. 60 seconds (1 minute) by default. Setting to 0 will disable the health check. - // +kubebuilder:default:=60 - HealthCheckIntervalSeconds *int `json:"healthCheckIntervalSeconds,omitempty"` + // The healthcheck configuration for the issuer. This configures the frequency at which the issuer will perform + // a health check to determine issuer's connectivity to Command instance. + // +kubebuilder:validation:Optional + HealthCheck *HealthCheckConfig `json:"healthcheck,omitempty"` // EnrollmentPatternId is the ID of the enrollment pattern to use. Supported in Keyfactor Command 25.1 and later. // If both enrollment pattern and certificate template are specified, enrollment pattern will take precedence. @@ -283,6 +284,15 @@ const ( ConditionUnknown ConditionStatus = "Unknown" ) +type HealthCheckConfig struct { + // Determines whether to the health check when the issuer is healthy. Default: true + Enabled bool `json:"enabled"` + + // The interval at which to health check the issuer when healthy. Defaults to 1 minute. Must not be less than "30s". + // +kubebuilder:validation:Optional + Interval *metav1.Duration `json:"interval"` +} + func init() { SchemeBuilder.Register(&Issuer{}, &IssuerList{}) } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 9555ca1..372708f 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ limitations under the License. package v1alpha1 import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -83,6 +84,26 @@ func (in *ClusterIssuerList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HealthCheckConfig) DeepCopyInto(out *HealthCheckConfig) { + *out = *in + if in.Interval != nil { + in, out := &in.Interval, &out.Interval + *out = new(v1.Duration) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HealthCheckConfig. +func (in *HealthCheckConfig) DeepCopy() *HealthCheckConfig { + if in == nil { + return nil + } + out := new(HealthCheckConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Issuer) DeepCopyInto(out *Issuer) { *out = *in @@ -164,10 +185,10 @@ func (in *IssuerList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IssuerSpec) DeepCopyInto(out *IssuerSpec) { *out = *in - if in.HealthCheckIntervalSeconds != nil { - in, out := &in.HealthCheckIntervalSeconds, &out.HealthCheckIntervalSeconds - *out = new(int) - **out = **in + if in.HealthCheck != nil { + in, out := &in.HealthCheck, &out.HealthCheck + *out = new(HealthCheckConfig) + (*in).DeepCopyInto(*out) } } diff --git a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml index 6e80de0..962a5ac 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml @@ -102,12 +102,22 @@ spec: Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. type: string - healthCheckIntervalSeconds: - default: 60 - description: The number of seconds between successful health checks. - 60 seconds (1 minute) by default. Setting to 0 will disable the - health check. - type: integer + healthcheck: + description: |- + The healthcheck configuration for the issuer. This configures the frequency at which the issuer will perform + a health check to determine issuer's connectivity to Command instance. + properties: + enabled: + description: 'Determines whether to the health check when the + issuer is healthy. Default: true' + type: boolean + interval: + description: The interval at which to health check the issuer + when healthy. Defaults to 1 minute. Must not be less than "30s". + type: string + required: + - enabled + type: object hostname: description: Hostname is the hostname of a Keyfactor Command instance. type: string diff --git a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml index 3bd45d3..329f48a 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml @@ -102,12 +102,22 @@ spec: Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. type: string - healthCheckIntervalSeconds: - default: 60 - description: The number of seconds between successful health checks. - 60 seconds (1 minute) by default. Setting to 0 will disable the - health check. - type: integer + healthcheck: + description: |- + The healthcheck configuration for the issuer. This configures the frequency at which the issuer will perform + a health check to determine issuer's connectivity to Command instance. + properties: + enabled: + description: 'Determines whether to the health check when the + issuer is healthy. Default: true' + type: boolean + interval: + description: The interval at which to health check the issuer + when healthy. Defaults to 1 minute. Must not be less than "30s". + type: string + required: + - enabled + type: object hostname: description: Hostname is the hostname of a Keyfactor Command instance. type: string diff --git a/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml b/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml index a0f8e5b..db32bf1 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml @@ -79,12 +79,22 @@ spec: Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. type: string - healthCheckIntervalSeconds: - default: 60 - description: The number of seconds between successful health checks. - 60 seconds (1 minute) by default. Setting to 0 will disable the - health check. - type: integer + healthcheck: + description: |- + The healthcheck configuration for the issuer. This configures the frequency at which the issuer will perform + a health check to determine issuer's connectivity to Command instance. + properties: + enabled: + description: 'Determines whether to the health check when the + issuer is healthy. Default: true' + type: boolean + interval: + description: The interval at which to health check the issuer + when healthy. Defaults to 1 minute. + type: string + required: + - enabled + type: object ownerRoleId: description: |- OwnerRoleId is the ID of the security role assigned as the certificate owner. diff --git a/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml b/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml index df6b81b..6490346 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml @@ -79,12 +79,22 @@ spec: Enrollment will fail if the specified template is not compatible with the enrollment pattern. Refer to the Keyfactor Command documentation for more information. type: string - healthCheckIntervalSeconds: - default: 60 - description: The number of seconds between successful health checks. - 60 seconds (1 minute) by default. Setting to 0 will disable the - health check. - type: integer + healthcheck: + description: |- + The healthcheck configuration for the issuer. This configures the frequency at which the issuer will perform + a health check to determine issuer's connectivity to Command instance. + properties: + enabled: + description: 'Determines whether to the health check when the + issuer is healthy. Default: true' + type: boolean + interval: + description: The interval at which to health check the issuer + when healthy. Defaults to 1 minute. + type: string + required: + - enabled + type: object ownerRoleId: description: |- OwnerRoleId is the ID of the security role assigned as the certificate owner. diff --git a/docsource/content.md b/docsource/content.md index d6fa661..f03506e 100644 --- a/docsource/content.md +++ b/docsource/content.md @@ -219,7 +219,9 @@ For example, ClusterIssuer resources can be used to issue certificates for resou | ownerRoleName | The name of the security role assigned as the certificate owner. The security role must be assigned to the identity context of the issuer. If `ownerRoleId` and `ownerRoleName` are both specified, `ownerRoleId` will take precedence. This field is **required** if the enrollment pattern, certificate template, or system-wide setting requires it. | | scopes | (Optional) Required if using ambient credentials with Azure AKS. If using ambient credentials, these scopes will be put on the access token generated by the ambient credentials' token provider, if applicable. | | audience | (Optional) If using ambient credentials, this audience will be put on the access token generated by the ambient credentials' token provider, if applicable. Google's ambient credential token provider generates an OIDC ID Token. If this value is not provided, it will default to `command`. | - | healthCheckIntervalSeconds | (Optional) Defines the health check interval, in seconds, for a healthy issuer. If ommitted, defaults to 60 seconds. If set to 0, it will disable the health check. If there is a failure when running the health check, it will retry in 10 seconds with an exponential backoff strategy. Value must not be negative. | + | healthcheck | (Optional) Defines the health check configuration for the issuer. If ommitted, health checks will be enabled and default to 60 seconds. If left disabled, the issuer will not perform a health check when the issuer is healthy and may cause CertificateRequest resources to silently fail. | + | healthcheck.enabled | (Required if health check block provided) Boolean to enable / disable health checks. By default, health checks are enabled. | + | healthcheck.interval | (Optional) Defines the interval between health checks. Example values: `30s`, `1m`, `5.5m`. To prevent overloading the Command instance, this interval must not be less than `30s`. Default value: `60s`. | > If a different combination of hostname/certificate authority/certificate template is required, a new Issuer or ClusterIssuer resource must be created. Each resource instantiation represents a single configuration. @@ -251,7 +253,9 @@ For example, ClusterIssuer resources can be used to issue certificates for resou # ownerRoleName: "$OWNER_ROLE_NAME" # Uncomment if required # scopes: "openid email https://example.com/.default" # Uncomment if required # audience: "https://your-command-url.com" # Uncomment if desired - # healthCheckIntervalSeconds: 60 # Uncomment if desired. Setting to 0 disables health check. + # healthcheck: # Optional health check configuration + # enabled: true + # interval: 30s EOF kubectl -n default apply -f issuer.yaml @@ -282,7 +286,9 @@ For example, ClusterIssuer resources can be used to issue certificates for resou # ownerRoleName: "$OWNER_ROLE_NAME" # Uncomment if required # scopes: "openid email https://example.com/.default" # Uncomment if required # audience: "https://your-command-url.com" # Uncomment if desired - # healthCheckIntervalSeconds: 60 # Uncomment if desired. Setting to 0 disables health check. + # healthcheck: # Optional health check configuration + # enabled: true + # interval: 30s EOF kubectl apply -f clusterissuer.yaml diff --git a/e2e/run_tests.sh b/e2e/run_tests.sh index 793774a..ade2dd2 100755 --- a/e2e/run_tests.sh +++ b/e2e/run_tests.sh @@ -43,7 +43,7 @@ IMAGE_TAG="local" # Uncomment if you want to build the image locally FULL_IMAGE_NAME="${IMAGE_REPO}/${IMAGE_NAME}:${IMAGE_TAG}" HELM_CHART_NAME="command-cert-manager-issuer" -#H ELM_CHART_VERSION="2.1.0" # Uncomment if you want to use a specific version from the Helm repository +# HELM_CHART_VERSION="2.1.0" # Uncomment if you want to use a specific version from the Helm repository HELM_CHART_VERSION="local" # Uncomment if you want to use the local Helm chart IS_LOCAL_DEPLOYMENT=$([ "$IMAGE_TAG" = "local" ] && echo "true" || echo "false") @@ -205,7 +205,7 @@ install_cert_manager_issuer() { CHART_PATH="command-issuer/command-cert-manager-issuer" echo "Using Helm chart from repository for version ${HELM_CHART_VERSION}: $CHART_PATH..." - VERSION_PARAM="--version ${HELM_CHART_VERSION}" + VERSION_PARAM="--version ${HELM_CHART_VERSION} --devel" fi # Only set the image repository parameter if we are deploying locally diff --git a/internal/controller/issuer_controller.go b/internal/controller/issuer_controller.go index a986130..62a889d 100644 --- a/internal/controller/issuer_controller.go +++ b/internal/controller/issuer_controller.go @@ -158,24 +158,31 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res func getHealthCheckInterval(log logr.Logger, issuer commandissuer.IssuerLike) (time.Duration, error) { spec := issuer.GetSpec() - if spec.HealthCheckIntervalSeconds == nil { - log.Info(fmt.Sprintf("health check spec value is nil, using default: %d", int(defaultHealthCheckInterval/time.Second))) + defaultInterval := int(defaultHealthCheckInterval / time.Second) + + if spec.HealthCheck == nil { + log.Info(fmt.Sprintf("health check spec value is nil, using default: %d", defaultInterval)) return defaultHealthCheckInterval, nil } - interval := *spec.HealthCheckIntervalSeconds + if !spec.HealthCheck.Enabled { + log.Info("health check has been disabled") + return 0, nil + } - // Health check interval should not be negative - if interval < 0 { - return 0, fmt.Errorf("interval %d is invalid, must be greater than or equal to 0", interval) + if spec.HealthCheck.Interval == nil { + log.Info(fmt.Sprintf("health check spec value is nil, using default: %d", defaultInterval)) + return defaultHealthCheckInterval, nil } - // Issuer may be configured to ignore future health checks - if interval == 0 { - log.Info("health check interval is configured to be 0. this will disable future health checks for issuer.") + healthCheckInterval := *spec.HealthCheck.Interval + + // To prevent from overloading the server, health check interval should not be less than 30 seconds + if healthCheckInterval.Duration < time.Duration(30)*time.Second { + return 0, fmt.Errorf("interval %s is invalid, must be greater than or equal to '30s'", healthCheckInterval) } - return time.Duration(interval) * time.Second, nil + return healthCheckInterval.Duration, nil } func commandConfigFromIssuer(ctx context.Context, c client.Client, issuer commandissuer.IssuerLike, secretNamespace string) (*command.Config, error) { diff --git a/internal/controller/issuer_controller_test.go b/internal/controller/issuer_controller_test.go index af952b8..707521b 100644 --- a/internal/controller/issuer_controller_test.go +++ b/internal/controller/issuer_controller_test.go @@ -578,8 +578,11 @@ func TestIssuerReconcile(t *testing.T) { Namespace: "ns1", }, Spec: commandissuerv1alpha1.IssuerSpec{ - SecretName: "issuer1-credentials", - HealthCheckIntervalSeconds: to.Ptr(30), + SecretName: "issuer1-credentials", + HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + Enabled: true, + Interval: to.Ptr(metav1.Duration{Duration: 30 * time.Second}), + }, }, Status: commandissuerv1alpha1.IssuerStatus{ Conditions: []commandissuerv1alpha1.IssuerCondition{ @@ -616,8 +619,11 @@ func TestIssuerReconcile(t *testing.T) { Name: "clusterissuer1", }, Spec: commandissuerv1alpha1.IssuerSpec{ - SecretName: "clusterissuer1-credentials", - HealthCheckIntervalSeconds: to.Ptr(30), + SecretName: "clusterissuer1-credentials", + HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + Enabled: true, + Interval: to.Ptr(metav1.Duration{Duration: 120 * time.Second}), + }, }, Status: commandissuerv1alpha1.IssuerStatus{ Conditions: []commandissuerv1alpha1.IssuerCondition{ @@ -644,9 +650,215 @@ func TestIssuerReconcile(t *testing.T) { clusterResourceNamespace: "kube-system", expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedResult: ctrl.Result{RequeueAfter: time.Duration(30) * time.Second}, + expectedResult: ctrl.Result{RequeueAfter: time.Duration(120) * time.Second}, + }, + "success-healthcheck-disabled": { + kind: "ClusterIssuer", + name: types.NamespacedName{Name: "clusterissuer1"}, + objects: []client.Object{ + &commandissuerv1alpha1.ClusterIssuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1", + }, + Spec: commandissuerv1alpha1.IssuerSpec{ + SecretName: "clusterissuer1-credentials", + HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + Enabled: false, + }, + }, + Status: commandissuerv1alpha1.IssuerStatus{ + Conditions: []commandissuerv1alpha1.IssuerCondition{ + { + Type: commandissuerv1alpha1.IssuerConditionReady, + Status: commandissuerv1alpha1.ConditionUnknown, + }, + }, + }, + }, + &corev1.Secret{ + Type: corev1.SecretTypeBasicAuth, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1-credentials", + Namespace: "kube-system", + }, + Data: map[string][]byte{ + corev1.BasicAuthUsernameKey: []byte("username"), + corev1.BasicAuthPasswordKey: []byte("password"), + }, + }, + }, + healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), + clusterResourceNamespace: "kube-system", + expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedResult: ctrl.Result{RequeueAfter: time.Duration(0)}, + }, + "success-no-healthcheck-interval": { + kind: "ClusterIssuer", + name: types.NamespacedName{Name: "clusterissuer1"}, + objects: []client.Object{ + &commandissuerv1alpha1.ClusterIssuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1", + }, + Spec: commandissuerv1alpha1.IssuerSpec{ + SecretName: "clusterissuer1-credentials", + HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + Enabled: true, + Interval: nil, + }, + }, + Status: commandissuerv1alpha1.IssuerStatus{ + Conditions: []commandissuerv1alpha1.IssuerCondition{ + { + Type: commandissuerv1alpha1.IssuerConditionReady, + Status: commandissuerv1alpha1.ConditionUnknown, + }, + }, + }, + }, + &corev1.Secret{ + Type: corev1.SecretTypeBasicAuth, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1-credentials", + Namespace: "kube-system", + }, + Data: map[string][]byte{ + corev1.BasicAuthUsernameKey: []byte("username"), + corev1.BasicAuthPasswordKey: []byte("password"), + }, + }, + }, + healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), + clusterResourceNamespace: "kube-system", + expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval}, }, - "error-healthcheck-negative-value": { + "success-nil-healthcheck-interval-defaults": { + kind: "ClusterIssuer", + name: types.NamespacedName{Name: "clusterissuer1"}, + objects: []client.Object{ + &commandissuerv1alpha1.ClusterIssuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1", + }, + Spec: commandissuerv1alpha1.IssuerSpec{ + SecretName: "clusterissuer1-credentials", + HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + Enabled: true, + Interval: nil, + }, + }, + Status: commandissuerv1alpha1.IssuerStatus{ + Conditions: []commandissuerv1alpha1.IssuerCondition{ + { + Type: commandissuerv1alpha1.IssuerConditionReady, + Status: commandissuerv1alpha1.ConditionUnknown, + }, + }, + }, + }, + &corev1.Secret{ + Type: corev1.SecretTypeBasicAuth, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1-credentials", + Namespace: "kube-system", + }, + Data: map[string][]byte{ + corev1.BasicAuthUsernameKey: []byte("username"), + corev1.BasicAuthPasswordKey: []byte("password"), + }, + }, + }, + healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), + clusterResourceNamespace: "kube-system", + expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedResult: ctrl.Result{RequeueAfter: time.Duration(60) * time.Second}, + }, + "success-nil-healthcheck-defaults": { + kind: "ClusterIssuer", + name: types.NamespacedName{Name: "clusterissuer1"}, + objects: []client.Object{ + &commandissuerv1alpha1.ClusterIssuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1", + }, + Spec: commandissuerv1alpha1.IssuerSpec{ + SecretName: "clusterissuer1-credentials", + HealthCheck: nil, + }, + Status: commandissuerv1alpha1.IssuerStatus{ + Conditions: []commandissuerv1alpha1.IssuerCondition{ + { + Type: commandissuerv1alpha1.IssuerConditionReady, + Status: commandissuerv1alpha1.ConditionUnknown, + }, + }, + }, + }, + &corev1.Secret{ + Type: corev1.SecretTypeBasicAuth, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1-credentials", + Namespace: "kube-system", + }, + Data: map[string][]byte{ + corev1.BasicAuthUsernameKey: []byte("username"), + corev1.BasicAuthPasswordKey: []byte("password"), + }, + }, + }, + healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), + clusterResourceNamespace: "kube-system", + expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedResult: ctrl.Result{RequeueAfter: time.Duration(60) * time.Second}, + }, + // "error-healthcheck-invalid-parsing": { + // kind: "Issuer", + // name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, + // objects: []client.Object{ + // &commandissuerv1alpha1.Issuer{ + // ObjectMeta: metav1.ObjectMeta{ + // Name: "issuer1", + // Namespace: "ns1", + // }, + // Spec: commandissuerv1alpha1.IssuerSpec{ + // SecretName: "issuer1-credentials", + // HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + // Enabled: true, + // Interval: to.Ptr(metav1.Duration{Duration: 30 * time.Second}), + // }, + // }, + // Status: commandissuerv1alpha1.IssuerStatus{ + // Conditions: []commandissuerv1alpha1.IssuerCondition{ + // { + // Type: commandissuerv1alpha1.IssuerConditionReady, + // Status: commandissuerv1alpha1.ConditionUnknown, + // }, + // }, + // }, + // }, + // &corev1.Secret{ + // Type: corev1.SecretTypeBasicAuth, + // ObjectMeta: metav1.ObjectMeta{ + // Name: "issuer1-credentials", + // Namespace: "ns1", + // }, + // Data: map[string][]byte{ + // corev1.BasicAuthUsernameKey: []byte("username"), + // corev1.BasicAuthPasswordKey: []byte("password"), + // }, + // }, + // }, + // healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false), + // expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, + // expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionUnknown, + // expectedResult: ctrl.Result{}, + // }, + "error-healthcheck-minimum-value": { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ @@ -656,8 +868,11 @@ func TestIssuerReconcile(t *testing.T) { Namespace: "ns1", }, Spec: commandissuerv1alpha1.IssuerSpec{ - SecretName: "issuer1-credentials", - HealthCheckIntervalSeconds: to.Ptr(-30), + SecretName: "issuer1-credentials", + HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + Enabled: true, + Interval: to.Ptr(metav1.Duration{Duration: 29 * time.Second}), + }, }, Status: commandissuerv1alpha1.IssuerStatus{ Conditions: []commandissuerv1alpha1.IssuerCondition{ From 974c9199f71d890dac4f2d444e8a03f32300327f Mon Sep 17 00:00:00 2001 From: Keyfactor Date: Mon, 27 Oct 2025 16:19:17 +0000 Subject: [PATCH 10/20] Update generated docs --- README.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0b3beef..376f2bf 100644 --- a/README.md +++ b/README.md @@ -251,7 +251,9 @@ For example, ClusterIssuer resources can be used to issue certificates for resou | ownerRoleName | The name of the security role assigned as the certificate owner. The security role must be assigned to the identity context of the issuer. If `ownerRoleId` and `ownerRoleName` are both specified, `ownerRoleId` will take precedence. This field is **required** if the enrollment pattern, certificate template, or system-wide setting requires it. | | scopes | (Optional) Required if using ambient credentials with Azure AKS. If using ambient credentials, these scopes will be put on the access token generated by the ambient credentials' token provider, if applicable. | | audience | (Optional) If using ambient credentials, this audience will be put on the access token generated by the ambient credentials' token provider, if applicable. Google's ambient credential token provider generates an OIDC ID Token. If this value is not provided, it will default to `command`. | - | healthCheckIntervalSeconds | (Optional) Defines the health check interval, in seconds, for a healthy issuer. If ommitted, defaults to 60 seconds. If set to 0, it will disable the health check. If there is a failure when running the health check, it will retry in 10 seconds with an exponential backoff strategy. Value must not be negative. | + | healthcheck | (Optional) Defines the health check configuration for the issuer. If ommitted, health checks will be enabled and default to 60 seconds. If left disabled, the issuer will not perform a health check when the issuer is healthy and may cause CertificateRequest resources to silently fail. | + | healthcheck.enabled | (Required if health check block provided) Boolean to enable / disable health checks. By default, health checks are enabled. | + | healthcheck.interval | (Optional) Defines the interval between health checks. Example values: `30s`, `1m`, `5.5m`. To prevent overloading the Command instance, this interval must not be less than `30s`. Default value: `60s`. | > If a different combination of hostname/certificate authority/certificate template is required, a new Issuer or ClusterIssuer resource must be created. Each resource instantiation represents a single configuration. @@ -283,7 +285,9 @@ For example, ClusterIssuer resources can be used to issue certificates for resou # ownerRoleName: "$OWNER_ROLE_NAME" # Uncomment if required # scopes: "openid email https://example.com/.default" # Uncomment if required # audience: "https://your-command-url.com" # Uncomment if desired - # healthCheckIntervalSeconds: 60 # Uncomment if desired. Setting to 0 disables health check. + # healthcheck: # Optional health check configuration + # enabled: true + # interval: 30s EOF kubectl -n default apply -f issuer.yaml @@ -314,7 +318,9 @@ For example, ClusterIssuer resources can be used to issue certificates for resou # ownerRoleName: "$OWNER_ROLE_NAME" # Uncomment if required # scopes: "openid email https://example.com/.default" # Uncomment if required # audience: "https://your-command-url.com" # Uncomment if desired - # healthCheckIntervalSeconds: 60 # Uncomment if desired. Setting to 0 disables health check. + # healthcheck: # Optional health check configuration + # enabled: true + # interval: 30s EOF kubectl apply -f clusterissuer.yaml From bb90adf57d2d9c6a1b15dd7140a0a45b823a962a Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Mon, 27 Oct 2025 12:42:37 -0400 Subject: [PATCH 11/20] chore: Change the specificaiton update to 2.4 Signed-off-by: Matthew H. Irby --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9539c3a..d4e62fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -# v2.3.3 +# v2.4.0 ## Features - Add a `healthcheck` specification to Issuer / ClusterIssuer resources, allowing flexibility in the health check interval. From 8d8ce73a013ad1b1aab5fe4195e8f4060b7ffc8c Mon Sep 17 00:00:00 2001 From: JSpon <115185500+JSpon@users.noreply.github.com> Date: Mon, 3 Nov 2025 08:51:19 -0500 Subject: [PATCH 12/20] Add the ability to specify the default issuer timeout across all issuers, if not specified. --- cmd/main.go | 17 +++++++++++++++++ .../command-cert-manager-issuer/README.md | 1 + .../templates/deployment.yaml | 3 +++ .../command-cert-manager-issuer/values.yaml | 2 ++ internal/controller/issuer_controller.go | 4 +++- internal/controller/issuer_controller_test.go | 1 + 6 files changed, 27 insertions(+), 1 deletion(-) diff --git a/cmd/main.go b/cmd/main.go index 51ca91e..1bd8617 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -22,6 +22,7 @@ import ( "flag" "fmt" "os" + "time" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. @@ -64,6 +65,7 @@ func main() { var metricsAddr string var enableLeaderElection bool var probeAddr string + var healthCheckInterval string var secureMetrics bool var enableHTTP2 bool var clusterResourceNamespace string @@ -79,6 +81,8 @@ func main() { "If set the metrics endpoint is served securely") flag.BoolVar(&enableHTTP2, "enable-http2", false, "If set, HTTP/2 will be enabled for the metrics and webhook servers") + flag.StringVar(&healthCheckInterval, "default-health-check-interval", "60s", + "If set, it is the default health check interval for issuers.") flag.StringVar(&clusterResourceNamespace, "cluster-resource-namespace", "", "The namespace for secrets in which cluster-scoped resources are found.") flag.BoolVar(&disableApprovedCheck, "disable-approved-check", false, "Disables waiting for CertificateRequests to have an approved condition before signing.") @@ -168,6 +172,17 @@ func main() { os.Exit(1) } + defaultHealthCheckInterval, err := time.ParseDuration(healthCheckInterval) + if err != nil { + setupLog.Error(err, "unable to parse default health check interval") + os.Exit(1) + } + + if defaultHealthCheckInterval < time.Duration(30) * time.Second { + setupLog.Error(err, fmt.Sprintf("interval %s is invalid, must be greater than or equal to '30s'", healthCheckInterval)) + os.Exit(1) + } + if err = (&controller.IssuerReconciler{ Client: mgr.GetClient(), Kind: "Issuer", @@ -175,6 +190,7 @@ func main() { SecretAccessGrantedAtClusterLevel: secretAccessGrantedAtClusterLevel, Scheme: mgr.GetScheme(), HealthCheckerBuilder: command.NewHealthChecker, + DefaultHealthCheckInterval: defaultHealthCheckInterval, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Issuer") os.Exit(1) @@ -186,6 +202,7 @@ func main() { ClusterResourceNamespace: clusterResourceNamespace, SecretAccessGrantedAtClusterLevel: secretAccessGrantedAtClusterLevel, HealthCheckerBuilder: command.NewHealthChecker, + DefaultHealthCheckInterval: defaultHealthCheckInterval, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterIssuer") os.Exit(1) diff --git a/deploy/charts/command-cert-manager-issuer/README.md b/deploy/charts/command-cert-manager-issuer/README.md index 5256fde..b26bb88 100644 --- a/deploy/charts/command-cert-manager-issuer/README.md +++ b/deploy/charts/command-cert-manager-issuer/README.md @@ -84,3 +84,4 @@ The following table lists the configurable parameters of the `command-cert-manag | `nodeSelector` | Node labels for pod assignment | `{}` | | `tolerations` | Tolerations for pod assignment | `[]` | | `secretConfig.useClusterRoleForSecretAccess` | Specifies if the ServiceAccount should be granted access to the Secret resource using a ClusterRole | `false` | +| `defaultHealthCheckInterval` | Specifies the default health check interval for issuers | `""` (uses the default in the code which is 60s) | diff --git a/deploy/charts/command-cert-manager-issuer/templates/deployment.yaml b/deploy/charts/command-cert-manager-issuer/templates/deployment.yaml index 856ace0..4725013 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/deployment.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/deployment.yaml @@ -36,6 +36,9 @@ spec: {{- if .Values.secretConfig.useClusterRoleForSecretAccess}} - --secret-access-granted-at-cluster-level {{- end}} + {{- if .Values.defaultHealthCheckInterval }} + - --default-health-check-interval={{ .Values.defaultHealthCheckInterval }} + {{- end }} command: - /manager image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.Version }}" diff --git a/deploy/charts/command-cert-manager-issuer/values.yaml b/deploy/charts/command-cert-manager-issuer/values.yaml index 6fd5bcb..4e8e7cc 100644 --- a/deploy/charts/command-cert-manager-issuer/values.yaml +++ b/deploy/charts/command-cert-manager-issuer/values.yaml @@ -70,3 +70,5 @@ resources: {} nodeSelector: {} tolerations: [] + +defaultHealthCheckInterval: "" diff --git a/internal/controller/issuer_controller.go b/internal/controller/issuer_controller.go index 62a889d..7ab4042 100644 --- a/internal/controller/issuer_controller.go +++ b/internal/controller/issuer_controller.go @@ -36,7 +36,6 @@ import ( const ( issuerReadyConditionReason = "command-issuer.IssuerController.Reconcile" - defaultHealthCheckInterval = time.Minute ) var ( @@ -44,6 +43,7 @@ var ( errGetCaSecret = errors.New("caSecretName specified a name, but failed to get Secret containing CA certificate") errHealthCheckerBuilder = errors.New("failed to build the healthchecker") errHealthCheckerCheck = errors.New("healthcheck failed") + defaultHealthCheckInterval = time.Minute ) // IssuerReconciler reconciles a Issuer object @@ -54,6 +54,7 @@ type IssuerReconciler struct { SecretAccessGrantedAtClusterLevel bool Scheme *runtime.Scheme HealthCheckerBuilder command.HealthCheckerBuilder + DefaultHealthCheckInterval time.Duration } //+kubebuilder:rbac:groups=command-issuer.keyfactor.com,resources=issuers;clusterissuers,verbs=get;list;watch @@ -67,6 +68,7 @@ func (r *IssuerReconciler) newIssuer() (commandissuer.IssuerLike, error) { if err != nil { return nil, err } + defaultHealthCheckInterval = r.DefaultHealthCheckInterval return ro.(commandissuer.IssuerLike), nil } diff --git a/internal/controller/issuer_controller_test.go b/internal/controller/issuer_controller_test.go index 707521b..38c3231 100644 --- a/internal/controller/issuer_controller_test.go +++ b/internal/controller/issuer_controller_test.go @@ -923,6 +923,7 @@ func TestIssuerReconcile(t *testing.T) { HealthCheckerBuilder: tc.healthCheckerBuilder, ClusterResourceNamespace: tc.clusterResourceNamespace, SecretAccessGrantedAtClusterLevel: true, + DefaultHealthCheckInterval: time.Minute, } result, err := controller.Reconcile( ctrl.LoggerInto(context.TODO(), logrtesting.NewTestLogger(t)), From dca6625ba01cb00d6cdb28f9fccbe40afd71b114 Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Tue, 11 Nov 2025 12:01:01 -0500 Subject: [PATCH 13/20] chore: address copilot feedback Signed-off-by: Matthew H. Irby --- api/v1alpha1/issuer_types.go | 2 +- .../command-issuer.keyfactor.com_clusterissuers.yaml | 2 +- .../bases/command-issuer.keyfactor.com_issuers.yaml | 2 +- .../templates/crds/clusterissuers.yaml | 2 +- .../templates/crds/issuers.yaml | 2 +- docsource/content.md | 2 +- internal/controller/issuer_controller.go | 10 +++++----- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/api/v1alpha1/issuer_types.go b/api/v1alpha1/issuer_types.go index f9f3329..dba03f9 100644 --- a/api/v1alpha1/issuer_types.go +++ b/api/v1alpha1/issuer_types.go @@ -285,7 +285,7 @@ const ( ) type HealthCheckConfig struct { - // Determines whether to the health check when the issuer is healthy. Default: true + // Determines whether to enable the health check when the issuer is healthy. Default: true Enabled bool `json:"enabled"` // The interval at which to health check the issuer when healthy. Defaults to 1 minute. Must not be less than "30s". diff --git a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml index 962a5ac..ab81afb 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml @@ -108,7 +108,7 @@ spec: a health check to determine issuer's connectivity to Command instance. properties: enabled: - description: 'Determines whether to the health check when the + description: 'Determines whether to enable the health check when the issuer is healthy. Default: true' type: boolean interval: diff --git a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml index 329f48a..3998533 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml @@ -108,7 +108,7 @@ spec: a health check to determine issuer's connectivity to Command instance. properties: enabled: - description: 'Determines whether to the health check when the + description: 'Determines whether to enable the health check when the issuer is healthy. Default: true' type: boolean interval: diff --git a/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml b/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml index db32bf1..f45d041 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml @@ -85,7 +85,7 @@ spec: a health check to determine issuer's connectivity to Command instance. properties: enabled: - description: 'Determines whether to the health check when the + description: 'Determines whether to enable the health check when the issuer is healthy. Default: true' type: boolean interval: diff --git a/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml b/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml index 6490346..10a5214 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml @@ -85,7 +85,7 @@ spec: a health check to determine issuer's connectivity to Command instance. properties: enabled: - description: 'Determines whether to the health check when the + description: 'Determines whether to enable the health check when the issuer is healthy. Default: true' type: boolean interval: diff --git a/docsource/content.md b/docsource/content.md index f03506e..366a699 100644 --- a/docsource/content.md +++ b/docsource/content.md @@ -219,7 +219,7 @@ For example, ClusterIssuer resources can be used to issue certificates for resou | ownerRoleName | The name of the security role assigned as the certificate owner. The security role must be assigned to the identity context of the issuer. If `ownerRoleId` and `ownerRoleName` are both specified, `ownerRoleId` will take precedence. This field is **required** if the enrollment pattern, certificate template, or system-wide setting requires it. | | scopes | (Optional) Required if using ambient credentials with Azure AKS. If using ambient credentials, these scopes will be put on the access token generated by the ambient credentials' token provider, if applicable. | | audience | (Optional) If using ambient credentials, this audience will be put on the access token generated by the ambient credentials' token provider, if applicable. Google's ambient credential token provider generates an OIDC ID Token. If this value is not provided, it will default to `command`. | - | healthcheck | (Optional) Defines the health check configuration for the issuer. If ommitted, health checks will be enabled and default to 60 seconds. If left disabled, the issuer will not perform a health check when the issuer is healthy and may cause CertificateRequest resources to silently fail. | + | healthcheck | (Optional) Defines the health check configuration for the issuer. If omitted, health checks will be enabled and default to 60 seconds. If left disabled, the issuer will not perform a health check when the issuer is healthy and may cause CertificateRequest resources to silently fail. | | healthcheck.enabled | (Required if health check block provided) Boolean to enable / disable health checks. By default, health checks are enabled. | | healthcheck.interval | (Optional) Defines the interval between health checks. Example values: `30s`, `1m`, `5.5m`. To prevent overloading the Command instance, this interval must not be less than `30s`. Default value: `60s`. | diff --git a/internal/controller/issuer_controller.go b/internal/controller/issuer_controller.go index 7ab4042..8057629 100644 --- a/internal/controller/issuer_controller.go +++ b/internal/controller/issuer_controller.go @@ -39,10 +39,10 @@ const ( ) var ( - errGetAuthSecret = errors.New("failed to get Secret containing Issuer credentials") - errGetCaSecret = errors.New("caSecretName specified a name, but failed to get Secret containing CA certificate") - errHealthCheckerBuilder = errors.New("failed to build the healthchecker") - errHealthCheckerCheck = errors.New("healthcheck failed") + errGetAuthSecret = errors.New("failed to get Secret containing Issuer credentials") + errGetCaSecret = errors.New("caSecretName specified a name, but failed to get Secret containing CA certificate") + errHealthCheckerBuilder = errors.New("failed to build the healthchecker") + errHealthCheckerCheck = errors.New("healthcheck failed") defaultHealthCheckInterval = time.Minute ) @@ -102,7 +102,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res healthCheckInterval, err := getHealthCheckInterval(log, issuer) if err != nil { - log.Error(err, "en error occurred while getting the health check interval") + log.Error(err, "an error occurred while getting the health check interval") issuer.GetStatus().SetCondition(ctx, commandissuer.IssuerConditionReady, commandissuer.ConditionFalse, issuerReadyConditionReason, err.Error()) issuer.GetStatus().SetCondition(ctx, commandissuer.IssuerConditionSupportsMetadata, commandissuer.ConditionUnknown, "", "") return ctrl.Result{}, nil From d3dc5a310cb98b9f28095868109cd00aa3f7b352 Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Tue, 11 Nov 2025 12:41:35 -0500 Subject: [PATCH 14/20] chore: fix autogenerated CRDs Signed-off-by: Matthew H. Irby --- .../bases/command-issuer.keyfactor.com_clusterissuers.yaml | 4 ++-- config/crd/bases/command-issuer.keyfactor.com_issuers.yaml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml index ab81afb..5d7a568 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml @@ -108,8 +108,8 @@ spec: a health check to determine issuer's connectivity to Command instance. properties: enabled: - description: 'Determines whether to enable the health check when the - issuer is healthy. Default: true' + description: 'Determines whether to enable the health check when + the issuer is healthy. Default: true' type: boolean interval: description: The interval at which to health check the issuer diff --git a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml index 3998533..3695476 100644 --- a/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml +++ b/config/crd/bases/command-issuer.keyfactor.com_issuers.yaml @@ -108,8 +108,8 @@ spec: a health check to determine issuer's connectivity to Command instance. properties: enabled: - description: 'Determines whether to enable the health check when the - issuer is healthy. Default: true' + description: 'Determines whether to enable the health check when + the issuer is healthy. Default: true' type: boolean interval: description: The interval at which to health check the issuer From ba5fb61aab25b07ba2974b55cab987d977f17fa4 Mon Sep 17 00:00:00 2001 From: Keyfactor Date: Tue, 11 Nov 2025 17:45:53 +0000 Subject: [PATCH 15/20] Update generated docs --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 376f2bf..c9e5fbe 100644 --- a/README.md +++ b/README.md @@ -251,7 +251,7 @@ For example, ClusterIssuer resources can be used to issue certificates for resou | ownerRoleName | The name of the security role assigned as the certificate owner. The security role must be assigned to the identity context of the issuer. If `ownerRoleId` and `ownerRoleName` are both specified, `ownerRoleId` will take precedence. This field is **required** if the enrollment pattern, certificate template, or system-wide setting requires it. | | scopes | (Optional) Required if using ambient credentials with Azure AKS. If using ambient credentials, these scopes will be put on the access token generated by the ambient credentials' token provider, if applicable. | | audience | (Optional) If using ambient credentials, this audience will be put on the access token generated by the ambient credentials' token provider, if applicable. Google's ambient credential token provider generates an OIDC ID Token. If this value is not provided, it will default to `command`. | - | healthcheck | (Optional) Defines the health check configuration for the issuer. If ommitted, health checks will be enabled and default to 60 seconds. If left disabled, the issuer will not perform a health check when the issuer is healthy and may cause CertificateRequest resources to silently fail. | + | healthcheck | (Optional) Defines the health check configuration for the issuer. If omitted, health checks will be enabled and default to 60 seconds. If left disabled, the issuer will not perform a health check when the issuer is healthy and may cause CertificateRequest resources to silently fail. | | healthcheck.enabled | (Required if health check block provided) Boolean to enable / disable health checks. By default, health checks are enabled. | | healthcheck.interval | (Optional) Defines the interval between health checks. Example values: `30s`, `1m`, `5.5m`. To prevent overloading the Command instance, this interval must not be less than `30s`. Default value: `60s`. | From dace4df39fd21b069d3c60bdbc76fccebf2b3cdd Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Tue, 11 Nov 2025 15:09:37 -0500 Subject: [PATCH 16/20] chore: address github copilot feedback Signed-off-by: Matthew H. Irby --- docsource/content.md | 2 ++ e2e/README.md | 2 +- e2e/run_tests.sh | 12 +++++++++--- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/docsource/content.md b/docsource/content.md index 366a699..4faaddb 100644 --- a/docsource/content.md +++ b/docsource/content.md @@ -112,6 +112,8 @@ Command Issuer is installed using a Helm chart. The chart is available in the [C --create-namespace ``` + > For all possible configuration values for the command-cert-manager-issuer Helm chart, please refer to [this list](./deploy/charts/command-cert-manager-issuer/README.md#configuration) + > The Helm chart installs the Command Issuer CRDs by default. The CRDs can be installed manually with the `make install` target. # Authentication diff --git a/e2e/README.md b/e2e/README.md index cc4ebc6..9a0262c 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -39,7 +39,7 @@ Modify the fields as needed. ```bash # enable the script to be executed -chmod +x ./run_test.sh +chmod +x ./run_tests.sh # load the environment variables source .env diff --git a/e2e/run_tests.sh b/e2e/run_tests.sh index ade2dd2..7541d35 100755 --- a/e2e/run_tests.sh +++ b/e2e/run_tests.sh @@ -205,7 +205,13 @@ install_cert_manager_issuer() { CHART_PATH="command-issuer/command-cert-manager-issuer" echo "Using Helm chart from repository for version ${HELM_CHART_VERSION}: $CHART_PATH..." - VERSION_PARAM="--version ${HELM_CHART_VERSION} --devel" + + # Only include --devel if HELM_CHART_VERSION is a pre-release (contains -alpha, -beta, -rc, etc.) + if [[ "${HELM_CHART_VERSION}" =~ -alpha|-beta|-rc ]]; then + VERSION_PARAM="--version ${HELM_CHART_VERSION} --devel" + else + VERSION_PARAM="--version ${HELM_CHART_VERSION}" + fi fi # Only set the image repository parameter if we are deploying locally @@ -482,10 +488,10 @@ delete_certificate() { echo "๐Ÿ—‘๏ธ Deleting certificate..." if cr_exists $CERTIFICATE_CRD_FQTN "$ISSUER_NAMESPACE" "$CR_C_NAME"; then - echo "Deleting Certificate called $CR_CR_NAME in $ISSUER_NAMESPACE" + echo "Deleting Certificate called $CR_C_NAME in $ISSUER_NAMESPACE" kubectl -n "$ISSUER_NAMESPACE" delete certificate "$CR_C_NAME" else - echo "โš ๏ธ Certificate $CR_CR_NAME not found in $ISSUER_NAMESPACE" + echo "โš ๏ธ Certificate $CR_C_NAME not found in $ISSUER_NAMESPACE" fi } From ea06d84a86f0690cebefbf60e1fdfe33aa436ea2 Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Tue, 11 Nov 2025 15:10:04 -0500 Subject: [PATCH 17/20] fix: resolve issue with default health check interval override Signed-off-by: Matthew H. Irby --- internal/controller/issuer_controller.go | 24 ++-- internal/controller/issuer_controller_test.go | 103 ++++++++++-------- 2 files changed, 66 insertions(+), 61 deletions(-) diff --git a/internal/controller/issuer_controller.go b/internal/controller/issuer_controller.go index 8057629..0bf2808 100644 --- a/internal/controller/issuer_controller.go +++ b/internal/controller/issuer_controller.go @@ -39,11 +39,10 @@ const ( ) var ( - errGetAuthSecret = errors.New("failed to get Secret containing Issuer credentials") - errGetCaSecret = errors.New("caSecretName specified a name, but failed to get Secret containing CA certificate") - errHealthCheckerBuilder = errors.New("failed to build the healthchecker") - errHealthCheckerCheck = errors.New("healthcheck failed") - defaultHealthCheckInterval = time.Minute + errGetAuthSecret = errors.New("failed to get Secret containing Issuer credentials") + errGetCaSecret = errors.New("caSecretName specified a name, but failed to get Secret containing CA certificate") + errHealthCheckerBuilder = errors.New("failed to build the healthchecker") + errHealthCheckerCheck = errors.New("healthcheck failed") ) // IssuerReconciler reconciles a Issuer object @@ -68,7 +67,6 @@ func (r *IssuerReconciler) newIssuer() (commandissuer.IssuerLike, error) { if err != nil { return nil, err } - defaultHealthCheckInterval = r.DefaultHealthCheckInterval return ro.(commandissuer.IssuerLike), nil } @@ -100,7 +98,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res } }() - healthCheckInterval, err := getHealthCheckInterval(log, issuer) + healthCheckInterval, err := r.getHealthCheckInterval(log, issuer) if err != nil { log.Error(err, "an error occurred while getting the health check interval") issuer.GetStatus().SetCondition(ctx, commandissuer.IssuerConditionReady, commandissuer.ConditionFalse, issuerReadyConditionReason, err.Error()) @@ -157,14 +155,14 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res return ctrl.Result{RequeueAfter: healthCheckInterval}, nil } -func getHealthCheckInterval(log logr.Logger, issuer commandissuer.IssuerLike) (time.Duration, error) { +func (r *IssuerReconciler) getHealthCheckInterval(log logr.Logger, issuer commandissuer.IssuerLike) (time.Duration, error) { spec := issuer.GetSpec() - defaultInterval := int(defaultHealthCheckInterval / time.Second) + defaultInterval := int(r.DefaultHealthCheckInterval / time.Second) if spec.HealthCheck == nil { - log.Info(fmt.Sprintf("health check spec value is nil, using default: %d", defaultInterval)) - return defaultHealthCheckInterval, nil + log.Info(fmt.Sprintf("health check spec value is nil, using default: %d seconds", defaultInterval)) + return r.DefaultHealthCheckInterval, nil } if !spec.HealthCheck.Enabled { @@ -173,8 +171,8 @@ func getHealthCheckInterval(log logr.Logger, issuer commandissuer.IssuerLike) (t } if spec.HealthCheck.Interval == nil { - log.Info(fmt.Sprintf("health check spec value is nil, using default: %d", defaultInterval)) - return defaultHealthCheckInterval, nil + log.Info(fmt.Sprintf("health check spec value is nil, using default: %d seconds", defaultInterval)) + return r.DefaultHealthCheckInterval, nil } healthCheckInterval := *spec.HealthCheck.Interval diff --git a/internal/controller/issuer_controller_test.go b/internal/controller/issuer_controller_test.go index 38c3231..7ccccbc 100644 --- a/internal/controller/issuer_controller_test.go +++ b/internal/controller/issuer_controller_test.go @@ -67,6 +67,7 @@ func TestIssuerReconcile(t *testing.T) { objects []client.Object healthCheckerBuilder command.HealthCheckerBuilder clusterResourceNamespace string + defaultHealthCheckInterval *time.Duration expectedResult ctrl.Result expectedError error expectedReadyConditionStatus commandissuerv1alpha1.ConditionStatus @@ -114,7 +115,7 @@ func TestIssuerReconcile(t *testing.T) { healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false), expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval}, + expectedResult: ctrl.Result{RequeueAfter: time.Minute}, }, "issuer-basicauth-no-username": { kind: "Issuer", @@ -236,7 +237,7 @@ func TestIssuerReconcile(t *testing.T) { clusterResourceNamespace: "kube-system", expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval}, + expectedResult: ctrl.Result{RequeueAfter: time.Minute}, }, "success-issuer-oauth": { kind: "Issuer", @@ -277,7 +278,7 @@ func TestIssuerReconcile(t *testing.T) { healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false), expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval}, + expectedResult: ctrl.Result{RequeueAfter: time.Minute}, }, "issuer-oauth-no-tokenurl": { kind: "Issuer", @@ -447,7 +448,7 @@ func TestIssuerReconcile(t *testing.T) { clusterResourceNamespace: "kube-system", expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval}, + expectedResult: ctrl.Result{RequeueAfter: time.Minute}, }, "issuer-kind-Unrecognized": { kind: "UnrecognizedType", @@ -733,7 +734,7 @@ func TestIssuerReconcile(t *testing.T) { clusterResourceNamespace: "kube-system", expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval}, + expectedResult: ctrl.Result{RequeueAfter: time.Minute}, }, "success-nil-healthcheck-interval-defaults": { kind: "ClusterIssuer", @@ -777,6 +778,46 @@ func TestIssuerReconcile(t *testing.T) { expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, expectedResult: ctrl.Result{RequeueAfter: time.Duration(60) * time.Second}, }, + "success-default-healthcheck-interval": { + kind: "ClusterIssuer", + name: types.NamespacedName{Name: "clusterissuer1"}, + objects: []client.Object{ + &commandissuerv1alpha1.ClusterIssuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1", + }, + Spec: commandissuerv1alpha1.IssuerSpec{ + SecretName: "clusterissuer1-credentials", + HealthCheck: nil, + }, + Status: commandissuerv1alpha1.IssuerStatus{ + Conditions: []commandissuerv1alpha1.IssuerCondition{ + { + Type: commandissuerv1alpha1.IssuerConditionReady, + Status: commandissuerv1alpha1.ConditionUnknown, + }, + }, + }, + }, + &corev1.Secret{ + Type: corev1.SecretTypeBasicAuth, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterissuer1-credentials", + Namespace: "kube-system", + }, + Data: map[string][]byte{ + corev1.BasicAuthUsernameKey: []byte("username"), + corev1.BasicAuthPasswordKey: []byte("password"), + }, + }, + }, + defaultHealthCheckInterval: to.Ptr(time.Duration(2) * time.Minute), + healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), + clusterResourceNamespace: "kube-system", + expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedResult: ctrl.Result{RequeueAfter: time.Duration(2) * time.Minute}, + }, "success-nil-healthcheck-defaults": { kind: "ClusterIssuer", name: types.NamespacedName{Name: "clusterissuer1"}, @@ -816,48 +857,6 @@ func TestIssuerReconcile(t *testing.T) { expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, expectedResult: ctrl.Result{RequeueAfter: time.Duration(60) * time.Second}, }, - // "error-healthcheck-invalid-parsing": { - // kind: "Issuer", - // name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, - // objects: []client.Object{ - // &commandissuerv1alpha1.Issuer{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "issuer1", - // Namespace: "ns1", - // }, - // Spec: commandissuerv1alpha1.IssuerSpec{ - // SecretName: "issuer1-credentials", - // HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ - // Enabled: true, - // Interval: to.Ptr(metav1.Duration{Duration: 30 * time.Second}), - // }, - // }, - // Status: commandissuerv1alpha1.IssuerStatus{ - // Conditions: []commandissuerv1alpha1.IssuerCondition{ - // { - // Type: commandissuerv1alpha1.IssuerConditionReady, - // Status: commandissuerv1alpha1.ConditionUnknown, - // }, - // }, - // }, - // }, - // &corev1.Secret{ - // Type: corev1.SecretTypeBasicAuth, - // ObjectMeta: metav1.ObjectMeta{ - // Name: "issuer1-credentials", - // Namespace: "ns1", - // }, - // Data: map[string][]byte{ - // corev1.BasicAuthUsernameKey: []byte("username"), - // corev1.BasicAuthPasswordKey: []byte("password"), - // }, - // }, - // }, - // healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false), - // expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, - // expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionUnknown, - // expectedResult: ctrl.Result{}, - // }, "error-healthcheck-minimum-value": { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, @@ -916,6 +915,13 @@ func TestIssuerReconcile(t *testing.T) { if tc.kind == "" { tc.kind = "Issuer" } + + defaultHealthcheckInterval := time.Minute + + if tc.defaultHealthCheckInterval != nil { + defaultHealthcheckInterval = *tc.defaultHealthCheckInterval + } + controller := IssuerReconciler{ Kind: tc.kind, Client: fakeClient, @@ -923,8 +929,9 @@ func TestIssuerReconcile(t *testing.T) { HealthCheckerBuilder: tc.healthCheckerBuilder, ClusterResourceNamespace: tc.clusterResourceNamespace, SecretAccessGrantedAtClusterLevel: true, - DefaultHealthCheckInterval: time.Minute, + DefaultHealthCheckInterval: defaultHealthcheckInterval, } + result, err := controller.Reconcile( ctrl.LoggerInto(context.TODO(), logrtesting.NewTestLogger(t)), reconcile.Request{NamespacedName: tc.name}, From 84e9be9176a2dfe74336296ca7312482838b8442 Mon Sep 17 00:00:00 2001 From: Keyfactor Date: Tue, 11 Nov 2025 20:13:07 +0000 Subject: [PATCH 18/20] Update generated docs --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index c9e5fbe..daa7636 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,8 @@ Command Issuer is installed using a Helm chart. The chart is available in the [C --create-namespace ``` + > For all possible configuration values for the command-cert-manager-issuer Helm chart, please refer to [this list](./deploy/charts/command-cert-manager-issuer/README.md#configuration) + > The Helm chart installs the Command Issuer CRDs by default. The CRDs can be installed manually with the `make install` target. # Authentication From befcbdec3d8ed63e3a28f7ff53206256710551c6 Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Tue, 11 Nov 2025 15:50:09 -0500 Subject: [PATCH 19/20] Update cmd/main.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- cmd/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main.go b/cmd/main.go index 1bd8617..3a35bab 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -179,7 +179,7 @@ func main() { } if defaultHealthCheckInterval < time.Duration(30) * time.Second { - setupLog.Error(err, fmt.Sprintf("interval %s is invalid, must be greater than or equal to '30s'", healthCheckInterval)) + setupLog.Error(errors.New(fmt.Sprintf("interval %s is invalid, must be greater than or equal to '30s'", healthCheckInterval)), "invalid health check interval") os.Exit(1) } From 7cbe047158e7ba4a6cc435d5409d3aa3f8edceae Mon Sep 17 00:00:00 2001 From: "Matthew H. Irby" Date: Wed, 12 Nov 2025 14:07:45 -0500 Subject: [PATCH 20/20] chore: fix some typos and serialization per recommendations Signed-off-by: Matthew H. Irby --- api/v1alpha1/issuer_types.go | 2 +- internal/controller/issuer_controller.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/issuer_types.go b/api/v1alpha1/issuer_types.go index dba03f9..418f47b 100644 --- a/api/v1alpha1/issuer_types.go +++ b/api/v1alpha1/issuer_types.go @@ -290,7 +290,7 @@ type HealthCheckConfig struct { // The interval at which to health check the issuer when healthy. Defaults to 1 minute. Must not be less than "30s". // +kubebuilder:validation:Optional - Interval *metav1.Duration `json:"interval"` + Interval *metav1.Duration `json:"interval,omitempty"` } func init() { diff --git a/internal/controller/issuer_controller.go b/internal/controller/issuer_controller.go index 0bf2808..262cc95 100644 --- a/internal/controller/issuer_controller.go +++ b/internal/controller/issuer_controller.go @@ -171,7 +171,7 @@ func (r *IssuerReconciler) getHealthCheckInterval(log logr.Logger, issuer comman } if spec.HealthCheck.Interval == nil { - log.Info(fmt.Sprintf("health check spec value is nil, using default: %d seconds", defaultInterval)) + log.Info(fmt.Sprintf("health check interval is nil, using default: %d seconds", defaultInterval)) return r.DefaultHealthCheckInterval, nil }