fix(load-balancer): wait for action of managed certificate (#1144)

The action from creating the managed certificate was not awaited, which
could cause issues where the creation fails, but this is not reported
back on the Service. We now await the action and report its status back
to the controller.

Author: apricote

hcloud/cloud.go

@@ -187,7 +187,7 @@ func (c *cloud) LoadBalancer() (cloudprovider.LoadBalancer, bool) {
 	lbOps := &hcops.LoadBalancerOps{
 		LBClient:      &c.client.LoadBalancer,
 		RobotClient:   c.robotClient,
-		CertOps:       &hcops.CertificateOps{CertClient: &c.client.Certificate},
+		CertOps:       &hcops.CertificateOps{ActionClient: &c.client.Action, CertClient: &c.client.Certificate},
 		ActionClient:  &c.client.Action,
 		NetworkClient: &c.client.Network,
 		NetworkID:     c.networkID,

internal/hcops/certificates.go

@@ -20,7 +20,8 @@ type HCloudCertificateClient interface {
 
 // CertificateOps implements all operations regarding Hetzner Cloud Certificates.
 type CertificateOps struct {
-	CertClient HCloudCertificateClient
+	ActionClient HCloudActionClient
+	CertClient   HCloudCertificateClient
 }
 
 // GetCertificateByNameOrID obtains a certificate from the Hetzner Cloud
@@ -81,12 +82,18 @@ func (co *CertificateOps) CreateManagedCertificate(
 		DomainNames: domains,
 		Labels:      labels,
 	}
-	_, _, err := co.CertClient.CreateCertificate(ctx, opts)
+	result, _, err := co.CertClient.CreateCertificate(ctx, opts)
 	if hcloud.IsError(err, hcloud.ErrorCodeUniquenessError) {
 		return fmt.Errorf("%s: %w", op, ErrAlreadyExists)
 	}
 	if err != nil {
 		return fmt.Errorf("%s: %w", op, err)
 	}
+
+	err = co.ActionClient.WaitFor(ctx, result.Action)
+	if err != nil {
+		return fmt.Errorf("%s: %w", op, err)
+	}
+
 	return nil
 }

internal/hcops/certificates_test.go

@@ -171,7 +171,7 @@ func TestCertificateOps_CreateManagedCertificate(t *testing.T) {
 		{
 			Name: "certificate creation successful",
 			Mock: func(_ *testing.T, tt *certificateOpsTestCase) {
-				res := hcloud.CertificateCreateResult{Certificate: &hcloud.Certificate{ID: 1}}
+				res := hcloud.CertificateCreateResult{Certificate: &hcloud.Certificate{ID: 1}, Action: &hcloud.Action{ID: 2}}
 				tt.CertClient.
 					On("CreateCertificate", tt.Ctx, hcloud.CertificateCreateOpts{
 						Name:        "test-cert",
@@ -180,6 +180,7 @@ func TestCertificateOps_CreateManagedCertificate(t *testing.T) {
 						Labels:      map[string]string{"key": "value"},
 					}).
 					Return(res, nil, nil)
+				tt.ActionClient.On("WaitFor", tt.Ctx, &hcloud.Action{ID: 2}).Return(nil)
 			},
 			Perform: func(t *testing.T, tt *certificateOpsTestCase) {
 				err := tt.CertOps.CreateManagedCertificate(
@@ -204,9 +205,10 @@ type certificateOpsTestCase struct {
 	ClientErr   error
 
 	// Set in run before actual test execution
-	Ctx        context.Context
-	CertOps    *hcops.CertificateOps
-	CertClient *mocks.CertificateClient
+	Ctx          context.Context
+	CertOps      *hcops.CertificateOps
+	CertClient   *mocks.CertificateClient
+	ActionClient *mocks.ActionClient
 }
 
 func (tt *certificateOpsTestCase) run(t *testing.T) {
@@ -215,7 +217,9 @@ func (tt *certificateOpsTestCase) run(t *testing.T) {
 	tt.Ctx = context.Background()
 	tt.CertClient = &mocks.CertificateClient{}
 	tt.CertClient.Test(t)
-	tt.CertOps = &hcops.CertificateOps{CertClient: tt.CertClient}
+	tt.ActionClient = &mocks.ActionClient{}
+	tt.ActionClient.Test(t)
+	tt.CertOps = &hcops.CertificateOps{ActionClient: tt.ActionClient, CertClient: tt.CertClient}
 
 	if tt.Mock != nil {
 		tt.Mock(t, tt)

internal/hcops/load_balancer_internal_test.go

@@ -29,7 +29,8 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 		mock               func(t *testing.T, tt *testCase)
 
 		// Set during test setup
-		certClient *mocks.CertificateClient
+		certClient   *mocks.CertificateClient
+		actionClient *mocks.ActionClient
 	}
 
 	tests := []testCase{
@@ -429,6 +430,8 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			tt.certClient = &mocks.CertificateClient{}
 			tt.certClient.Test(t)
+			tt.actionClient = &mocks.ActionClient{}
+			tt.actionClient.Test(t)
 
 			if tt.mock != nil {
 				tt.mock(t, &tt)
@@ -442,7 +445,7 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 						Annotations: map[string]string{},
 					},
 				},
-				CertOps: &CertificateOps{CertClient: tt.certClient},
+				CertOps: &CertificateOps{ActionClient: tt.actionClient, CertClient: tt.certClient},
 				cfg:     tt.cfg,
 			}
 			for k, v := range tt.serviceAnnotations {

internal/hcops/testing.go

@@ -46,7 +46,7 @@ func NewLoadBalancerOpsFixture(t *testing.T) *LoadBalancerOpsFixture {
 
 	fx.LBOps = &LoadBalancerOps{
 		LBClient:      fx.LBClient,
-		CertOps:       &CertificateOps{CertClient: fx.CertClient},
+		CertOps:       &CertificateOps{ActionClient: fx.ActionClient, CertClient: fx.CertClient},
 		ActionClient:  fx.ActionClient,
 		NetworkClient: fx.NetworkClient,
 		RobotClient:   fx.RobotClient,