diff --git a/adcs/ntlm_certsrv.go b/adcs/ntlm_certsrv.go index 0092f19..92aed7e 100644 --- a/adcs/ntlm_certsrv.go +++ b/adcs/ntlm_certsrv.go @@ -6,7 +6,7 @@ import ( "crypto/x509" "errors" "fmt" - "io/ioutil" + "io" "net/http" neturl "net/url" "os" @@ -19,10 +19,10 @@ import ( ) type NtlmCertsrv struct { - url string - username string - password string - ca string + url string + username string + password string + //ca string httpClient *http.Client } @@ -142,10 +142,10 @@ func (s *NtlmCertsrv) GetExistingCertificate(id string) (AdcsResponseStatus, str defer res.Body.Close() if res.StatusCode == http.StatusOK { - switch ct := strings.Split(res.Header.Get(http.CanonicalHeaderKey("content-type")), ";"); ct[0] { + switch ct := strings.Split(res.Header.Get("content-type"), ";"); ct[0] { case ct_html: // Denied or pending - body, err := ioutil.ReadAll(res.Body) + body, err := io.ReadAll(res.Body) if err != nil { log.Error(err, "Cannot read ADCS Certserv response") return certStatus, "", id, err @@ -192,7 +192,7 @@ func (s *NtlmCertsrv) GetExistingCertificate(id string) (AdcsResponseStatus, str case ct_pkix: // Certificate - cert, err := ioutil.ReadAll(res.Body) + cert, err := io.ReadAll(res.Body) if err != nil { log.Error(err, "Cannot read ADCS Certserv response") return certStatus, "", id, err @@ -267,7 +267,7 @@ func (s *NtlmCertsrv) RequestCertificate(csr string, template string) (AdcsRespo log.Info("Sending request", "response", res) } - body, err := ioutil.ReadAll(res.Body) + body, err := io.ReadAll(res.Body) log.Info("Body", "body", body) @@ -341,7 +341,7 @@ func (s *NtlmCertsrv) obtainCaCertificate(certPage string, expectedContentType s return "", err } defer res1.Body.Close() - body, err := ioutil.ReadAll(res1.Body) + body, err := io.ReadAll(res1.Body) if err != nil { log.Error(err, "Cannot read ADCS Certserv response") return "", err @@ -373,13 +373,13 @@ func (s *NtlmCertsrv) obtainCaCertificate(certPage string, expectedContentType s defer res2.Body.Close() if res2.StatusCode == http.StatusOK { - ct := res2.Header.Get(http.CanonicalHeaderKey("content-type")) + ct := res2.Header.Get("content-type") if expectedContentType != ct { err = errors.New("Unexpected content type") log.Error(err, err.Error(), "content type", ct) return "", err } - body, err := ioutil.ReadAll(res2.Body) + body, err := io.ReadAll(res2.Body) if err != nil { log.Error(err, "Cannot read ADCS Certserv response") return "", err diff --git a/charts/adcs-issuer/Chart.yaml b/charts/adcs-issuer/Chart.yaml index b8b1ab7..429514d 100644 --- a/charts/adcs-issuer/Chart.yaml +++ b/charts/adcs-issuer/Chart.yaml @@ -13,10 +13,10 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 2.1.1 +version: 2.1.2 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. # It is recommended to use it with quotes. -appVersion: "2.1.1" +appVersion: "2.1.2" diff --git a/charts/adcs-issuer/templates/deployment.yaml b/charts/adcs-issuer/templates/deployment.yaml index d4af9b4..c47d2fa 100644 --- a/charts/adcs-issuer/templates/deployment.yaml +++ b/charts/adcs-issuer/templates/deployment.yaml @@ -77,7 +77,7 @@ spec: readOnlyRootFilesystem: true capabilities: drop: - - all + - ALL volumeMounts: diff --git a/charts/adcs-issuer/values.yaml b/charts/adcs-issuer/values.yaml index 713e314..7c564d9 100644 --- a/charts/adcs-issuer/values.yaml +++ b/charts/adcs-issuer/values.yaml @@ -8,7 +8,7 @@ controllerManager: manager: image: repository: djkormo/adcs-issuer - tag: 2.1.0 + tag: 2.1.2 resources: limits: cpu: 100m @@ -79,7 +79,7 @@ nodeSelector: {} # ADCS Simulator simulator: - enabled: true + enabled: false clusterIssuserName: adcs-sim-adcsclusterissuer deploymentName: adcs-sim-deployment configMapName: adcs-sim-configmap @@ -125,7 +125,7 @@ simulator: readOnlyRootFilesystem: true capabilities: drop: - - all + - ALL resources: diff --git a/controllers/adcsrequest_controller.go b/controllers/adcsrequest_controller.go index 8474e54..02366d7 100644 --- a/controllers/adcsrequest_controller.go +++ b/controllers/adcsrequest_controller.go @@ -91,11 +91,20 @@ func (r *AdcsRequestReconciler) Reconcile(ctx context.Context, req ctrl.Request) // Get the original CertificateRequest to set result in cr, err := r.CertificateRequestController.GetCertificateRequest(ctx, req.NamespacedName) + if err != nil { + log.Error(err, "Failed request will be re-tried", "retry interval", issuer.RetryInterval) + return ctrl.Result{Requeue: true, RequeueAfter: issuer.RetryInterval}, nil + } + switch ar.Status.State { case api.Pending: // Check again later log.Info(fmt.Sprintf("Pending request will be re-tried in %v", issuer.StatusCheckInterval)) - r.setStatus(ctx, ar) + err = r.setStatus(ctx, ar) + if err != nil { + log.Error(err, "Failed request will be re-tried", "retry interval", issuer.RetryInterval) + return ctrl.Result{Requeue: true, RequeueAfter: issuer.RetryInterval}, nil + } return ctrl.Result{Requeue: true, RequeueAfter: issuer.StatusCheckInterval}, nil case api.Ready: @@ -113,18 +122,36 @@ func (r *AdcsRequestReconciler) Reconcile(ctx context.Context, req ctrl.Request) // CA cert is inside the cert above // cr.Status.CA = caCert - r.CertificateRequestController.SetStatus(ctx, &cr, cmmeta.ConditionTrue, cmapi.CertificateRequestReasonIssued, "ADCS request successful") + err = r.CertificateRequestController.SetStatus(ctx, &cr, cmmeta.ConditionTrue, cmapi.CertificateRequestReasonIssued, "ADCS request successful") + if err != nil { + log.Error(err, "Failed request will be re-tried", "retry interval", issuer.RetryInterval) + return ctrl.Result{Requeue: true, RequeueAfter: issuer.RetryInterval}, nil + } case api.Rejected: // This is a little hack for strange cert-manager behavior in case of failed request. Cert-manager automatically // re-tries such requests (re-created CertificateRequest object) what doesn't make sense in case of rejection. // We keep the Reason 'Pending' to prevent from re-trying while the actual status is in the Status Condition's Message field. // TODO: change it when cert-manager handles this better. - r.CertificateRequestController.SetStatus(ctx, &cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "ADCS request rejected") + err = r.CertificateRequestController.SetStatus(ctx, &cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "ADCS request rejected") + if err != nil { + log.Error(err, "Failed request will be re-tried", "retry interval", issuer.RetryInterval) + return ctrl.Result{Requeue: true, RequeueAfter: issuer.RetryInterval}, nil + } + case api.Errored: - r.CertificateRequestController.SetStatus(ctx, &cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "ADCS request errored") + err = r.CertificateRequestController.SetStatus(ctx, &cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "ADCS request errored") + if err != nil { + log.Error(err, "Failed request will be re-tried", "retry interval", issuer.RetryInterval) + return ctrl.Result{Requeue: true, RequeueAfter: issuer.RetryInterval}, nil + } + } + + err = r.setStatus(ctx, ar) + if err != nil { + log.Error(err, "Failed request will be re-tried", "retry interval", issuer.RetryInterval) + return ctrl.Result{Requeue: true, RequeueAfter: issuer.RetryInterval}, nil } - r.setStatus(ctx, ar) return ctrl.Result{}, nil } diff --git a/controllers/certificaterequest_controller.go b/controllers/certificaterequest_controller.go index 63c16b9..d629570 100644 --- a/controllers/certificaterequest_controller.go +++ b/controllers/certificaterequest_controller.go @@ -161,7 +161,11 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R if err != nil { return ctrl.Result{}, err } - r.SetStatus(ctx, &cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Processing ADCS request") + + err = r.SetStatus(ctx, &cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Processing ADCS request") + if err != nil { + return ctrl.Result{}, err + } log.V(4).Info("setstatus", "ctx", ctx, "cr", &cr) diff --git a/deploy.env b/deploy.env index 24ea999..f9fe938 100644 --- a/deploy.env +++ b/deploy.env @@ -1,6 +1,6 @@ APP_NAME=adcs-issuer DOCKER_REPO=djkormo -VERSION=2.1.1 +VERSION=2.1.2 diff --git a/issuers/issuer_factory.go b/issuers/issuer_factory.go index a182b06..4c80d44 100644 --- a/issuers/issuer_factory.go +++ b/issuers/issuer_factory.go @@ -64,7 +64,7 @@ func (f *IssuerFactory) getAdcsIssuer(ctx context.Context, key client.ObjectKey) caCertPool := x509.NewCertPool() ok := caCertPool.AppendCertsFromPEM(certs) - if ok == false { + if !ok { return nil, fmt.Errorf("error loading ADCS CA bundle") } @@ -117,7 +117,7 @@ func (f *IssuerFactory) getClusterAdcsIssuer(ctx context.Context, key client.Obj caCertPool := x509.NewCertPool() ok := caCertPool.AppendCertsFromPEM(certs) - if ok == false { + if !ok { return nil, fmt.Errorf("error loading ADCS CA bundle") } diff --git a/issuers/issuer_test.go b/issuers/issuer_test.go index e31df55..081f987 100644 --- a/issuers/issuer_test.go +++ b/issuers/issuer_test.go @@ -1,10 +1,10 @@ package issuers import ( - "io/ioutil" - "testing" - + "fmt" "github.com/stretchr/testify/assert" + "os" + "testing" ctrl "sigs.k8s.io/controller-runtime" ) @@ -20,10 +20,10 @@ var ( func TestParsingCaCertShouldReturnX509(t *testing.T) { // arrange - pkcs7Pem, err := ioutil.ReadFile("testdata/pkcs7.pem") + pkcs7Pem, err := os.ReadFile("testdata/pkcs7.pem") assert.NoError(t, err) - validX509Certificate, err := ioutil.ReadFile("testdata/x509.pem") + validX509Certificate, err := os.ReadFile("testdata/x509.pem") assert.NoError(t, err) // act @@ -36,7 +36,7 @@ func TestParsingCaCertShouldReturnX509(t *testing.T) { func TestIncorrectFormatPkcs(t *testing.T) { //arrange - incorrectPKCS7Cert, err := ioutil.ReadFile("testdata/incorrectPKCS7Cert.pem") + incorrectPKCS7Cert, err := os.ReadFile("testdata/incorrectPKCS7Cert.pem") assert.NoError(t, err) // act @@ -78,9 +78,9 @@ func TestIncorrectCertFormat(t *testing.T) { func TestParseCaCertCorrectPKCS7(t *testing.T) { // arrange // raw format pkcs7.p7b from cfss testdata (https://github.com/cloudflare/cfssl/tree/master/helpers/testdata) - rawPkcs7, err := ioutil.ReadFile("testdata/cfss_rawPKCS7.p7b") + rawPkcs7, err := os.ReadFile("testdata/cfss_rawPKCS7.p7b") assert.NoError(t, err) - cfssOutputX509, err := ioutil.ReadFile("testdata/cfss_outputx509.pem") + cfssOutputX509, err := os.ReadFile("testdata/cfss_outputx509.pem") assert.NoError(t, err) // act @@ -95,8 +95,11 @@ func TestParseCaCertCorrectPKCS7(t *testing.T) { func TestCorrectX509Cert(t *testing.T) { // arrange // raw format pkcs7.p7b from cfss testdata (https://github.com/cloudflare/cfssl/tree/master/helpers/testdata) - x509, err := ioutil.ReadFile("testdata/x509.pem") + x509, err := os.ReadFile("testdata/x509.pem") + if err != nil { + fmt.Println("TestCorrectX509Cert") + } // act parsedCaCert, err := parseCaCert(x509, log) diff --git a/main.go b/main.go index 67300e3..833dce3 100644 --- a/main.go +++ b/main.go @@ -124,8 +124,18 @@ func main() { os.Exit(1) } - mgr.AddHealthzCheck("healthz", healthcheck.HealthCheck) - mgr.AddReadyzCheck("readyz", healthcheck.HealthCheck) + err = mgr.AddHealthzCheck("healthz", healthcheck.HealthCheck) + if err != nil { + setupLog.Error(err, "unable to start AddHealthzCheck") + os.Exit(1) + } + + err = mgr.AddReadyzCheck("readyz", healthcheck.HealthCheck) + if err != nil { + setupLog.Error(err, "unable to start AddReadyzCheck") + os.Exit(1) + } + certificateRequestReconciler := &controllers.CertificateRequestReconciler{ Client: mgr.GetClient(), Recorder: mgr.GetEventRecorderFor("adcs-certificaterequests-controller"),