From ba0de871ac352533acd9ae97970f4522599f35a5 Mon Sep 17 00:00:00 2001 From: Luis Presuel Date: Thu, 21 Nov 2024 10:24:11 -0600 Subject: [PATCH 1/3] removes unneeded set of 10 seconds for timeout in tests. Is not needed and rather just produces sporadic errors in pipeline if TPP is not fast enough --- pkg/venafi/tpp/connector_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/venafi/tpp/connector_test.go b/pkg/venafi/tpp/connector_test.go index 8604f696..00fdbad6 100644 --- a/pkg/venafi/tpp/connector_test.go +++ b/pkg/venafi/tpp/connector_test.go @@ -1814,7 +1814,6 @@ func TestRenewCertRestoringValues(t *testing.T) { req.KeyType = certificate.KeyTypeECDSA req.KeyCurve = certificate.EllipticCurveP521 req.CsrOrigin = certificate.ServiceGeneratedCSR - req.Timeout = time.Second * 10 err = tpp.GenerateRequest(&endpoint.ZoneConfiguration{}, req) if err != nil { t.Fatalf("err is not nil, err: %s", err) @@ -2288,7 +2287,6 @@ func TestEnrollWithLocation(t *testing.T) { req := certificate.Request{} req.Subject.CommonName = cn - req.Timeout = time.Second * 10 req.Location = &certificate.Location{ Instance: "instance", Workload: workload, @@ -2305,7 +2303,6 @@ func TestEnrollWithLocation(t *testing.T) { } req = certificate.Request{} req.Subject.CommonName = cn - req.Timeout = time.Second * 10 req.Location = &certificate.Location{ Instance: "instance", Workload: workload, @@ -2322,7 +2319,6 @@ func TestEnrollWithLocation(t *testing.T) { } req = certificate.Request{} req.Subject.CommonName = cn - req.Timeout = time.Second * 10 req.Location = &certificate.Location{ Instance: "instance", Workload: workload, @@ -2880,7 +2876,6 @@ func TestCreateSshCertServiceGeneratedKP(t *testing.T) { req.ValidityPeriod = fmt.Sprint(duration, "h") req.Template = os.Getenv("TPP_SSH_CA") req.SourceAddresses = []string{"test.com"} - req.Timeout = time.Second * 10 respData, err := tpp.RequestSSHCertificate(req) @@ -2959,7 +2954,6 @@ func TestCreateSshCertLocalGeneratedKP(t *testing.T) { req.ValidityPeriod = fmt.Sprint(duration, "h") req.Template = os.Getenv("TPP_SSH_CA") req.SourceAddresses = []string{"test.com"} - req.Timeout = time.Second * 10 sPubKey := string(pub) From 19f69034f6d85c97d33fa837ce0875e7c7e244f7 Mon Sep 17 00:00:00 2001 From: Luis Presuel Date: Thu, 21 Nov 2024 11:25:41 -0600 Subject: [PATCH 2/3] enhances way of SSH flow to deal with set timeout in Request. Removes request set timeout in some tests since that now VCert it uses it to set workToDoTimeout, it will give us errors when the tiemout is very small. Now we use TPP's default value for it --- pkg/venafi/tpp/connector_test.go | 9 +++++++-- pkg/venafi/tpp/sshCertUtils.go | 6 ++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/venafi/tpp/connector_test.go b/pkg/venafi/tpp/connector_test.go index 00fdbad6..69e97c2d 100644 --- a/pkg/venafi/tpp/connector_test.go +++ b/pkg/venafi/tpp/connector_test.go @@ -1827,6 +1827,11 @@ func TestRenewCertRestoringValues(t *testing.T) { req.FetchPrivateKey = true req.KeyPassword = os.Getenv("TPP_PASSWORD") + req.Timeout = time.Second * 30 // explicitly setting this value here + // to make sure we only wait for certificate issuance for 30 seconds because + // setting it before the RequestCertificate function, will override + // workToDoTimeout to only wait to 30 seconds + pcc, err := tpp.RetrieveCertificate(req) if err != nil { t.Fatal(err) @@ -1846,11 +1851,11 @@ func TestRenewCertRestoringValues(t *testing.T) { renewReq := certificate.RenewalRequest{ CertificateDN: req.PickupID, } - pickupdID, err := tpp.RenewCertificate(&renewReq) + pickupID, err := tpp.RenewCertificate(&renewReq) if err != nil { t.Fatal(err) } - req = &certificate.Request{PickupID: pickupdID, Timeout: 30 * time.Second} + req = &certificate.Request{PickupID: pickupID, Timeout: 30 * time.Second} pcc, err = tpp.RetrieveCertificate(req) if err != nil { t.Fatal(err) diff --git a/pkg/venafi/tpp/sshCertUtils.go b/pkg/venafi/tpp/sshCertUtils.go index ce2a234b..c067674b 100644 --- a/pkg/venafi/tpp/sshCertUtils.go +++ b/pkg/venafi/tpp/sshCertUtils.go @@ -46,7 +46,9 @@ func RequestSshCertificate(c *Connector, req *certificate.SshCertRequest) (*cert } //TODO: Maybe, there is a better way to set the timeout. - c.client.Timeout = time.Duration(req.Timeout) * time.Second + if req.Timeout > 0 { + c.client.Timeout = req.Timeout * time.Second + } statusCode, status, body, err := c.request("POST", urlResourceSshCertReq, sshCertReq) if err != nil { return nil, err @@ -55,7 +57,7 @@ func RequestSshCertificate(c *Connector, req *certificate.SshCertRequest) (*cert response, err := parseSshCertOperationResponse(statusCode, status, body) if err != nil { - if response.Response.ErrorMessage != "" && c.verbose { + if &response != nil && response.Response.ErrorMessage != "" && c.verbose { log.Println(util.GetJsonAsString(response.Response)) } return nil, err From 7db073f7a33f2db8b773c57bd5ae6e41be5ab2e3 Mon Sep 17 00:00:00 2001 From: Luis Presuel Date: Thu, 21 Nov 2024 11:45:10 -0600 Subject: [PATCH 3/3] adds missing error checking in tests. Reverts validation for SSH response to previous one --- pkg/venafi/tpp/connector_test.go | 7 +++++++ pkg/venafi/tpp/sshCertUtils.go | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/venafi/tpp/connector_test.go b/pkg/venafi/tpp/connector_test.go index 69e97c2d..03428a6e 100644 --- a/pkg/venafi/tpp/connector_test.go +++ b/pkg/venafi/tpp/connector_test.go @@ -2863,6 +2863,9 @@ func TestSetPolicyValuesAndValidate(t *testing.T) { func TestCreateSshCertServiceGeneratedKP(t *testing.T) { tpp, err := getTestConnector(ctx.TPPurl, ctx.TPPZone) + if err != nil { + t.Fatalf("err is not nil, err: %s", err) + } duration := 4 @@ -2928,6 +2931,10 @@ func TestCreateSshCertLocalGeneratedKP(t *testing.T) { tpp, err := getTestConnector(ctx.TPPurl, ctx.TPPZone) + if err != nil { + t.Fatalf("err is not nil, err: %s", err) + } + duration := 4 tpp.verbose = true diff --git a/pkg/venafi/tpp/sshCertUtils.go b/pkg/venafi/tpp/sshCertUtils.go index c067674b..55980264 100644 --- a/pkg/venafi/tpp/sshCertUtils.go +++ b/pkg/venafi/tpp/sshCertUtils.go @@ -57,7 +57,7 @@ func RequestSshCertificate(c *Connector, req *certificate.SshCertRequest) (*cert response, err := parseSshCertOperationResponse(statusCode, status, body) if err != nil { - if &response != nil && response.Response.ErrorMessage != "" && c.verbose { + if response.Response.ErrorMessage != "" && c.verbose { log.Println(util.GetJsonAsString(response.Response)) } return nil, err