diff --git a/internal/pkg/rpaas/certificates.go b/internal/pkg/rpaas/certificates.go index 7fe0f4a4..4daa82f4 100644 --- a/internal/pkg/rpaas/certificates.go +++ b/internal/pkg/rpaas/certificates.go @@ -72,6 +72,7 @@ func (m *k8sRpaasManager) UpdateCertManagerRequest(ctx context.Context, instance } newRequest := v1alpha1.CertManager{ + Name: in.Name, Issuer: issuer, DNSNames: in.DNSNames, IPAddresses: in.IPAddresses, @@ -81,7 +82,9 @@ func (m *k8sRpaasManager) UpdateCertManagerRequest(ctx context.Context, instance instance.Spec.DynamicCertificates.CertManager = nil } - if index, found := findCertManagerRequestByIssuer(instance, in.Issuer); found { + if index, found := findCertManagerRequestByName(instance, in.Name); found { + instance.Spec.DynamicCertificates.CertManagerRequests[index] = newRequest + } else if index, found := findCertManagerRequestByIssuer(instance, in.Issuer); found { instance.Spec.DynamicCertificates.CertManagerRequests[index] = newRequest } else { instance.Spec.DynamicCertificates.CertManagerRequests = append(instance.Spec.DynamicCertificates.CertManagerRequests, newRequest) @@ -90,7 +93,7 @@ func (m *k8sRpaasManager) UpdateCertManagerRequest(ctx context.Context, instance return m.cli.Update(ctx, instance) } -func (m *k8sRpaasManager) DeleteCertManagerRequest(ctx context.Context, instanceName, issuer string) error { +func (m *k8sRpaasManager) DeleteCertManagerRequestByIssuer(ctx context.Context, instanceName, issuer string) error { instance, err := m.GetInstance(ctx, instanceName) if err != nil { return err @@ -123,6 +126,38 @@ func (m *k8sRpaasManager) DeleteCertManagerRequest(ctx context.Context, instance return m.cli.Update(ctx, instance) } +func (m *k8sRpaasManager) DeleteCertManagerRequestByName(ctx context.Context, instanceName, name string) error { + instance, err := m.GetInstance(ctx, instanceName) + if err != nil { + return err + } + + if name == "" { + return &ValidationError{Msg: "cert-manager name cannot be empty"} + } + + if instance.Spec.DynamicCertificates == nil { + instance.Spec.DynamicCertificates = &v1alpha1.DynamicCertificates{} + } + + if req := instance.Spec.DynamicCertificates.CertManager; req != nil { + if req.Name == name { + instance.Spec.DynamicCertificates.CertManager = nil + return m.cli.Update(ctx, instance) + } + } + + index, found := findCertManagerRequestByName(instance, name) + if !found { + return &NotFoundError{Msg: "cert-manager certificate has already been removed"} + } + + // NOTE: removes the index-th element of slice. + instance.Spec.DynamicCertificates.CertManagerRequests = append(instance.Spec.DynamicCertificates.CertManagerRequests[:index], instance.Spec.DynamicCertificates.CertManagerRequests[index+1:]...) + + return m.cli.Update(ctx, instance) +} + func (m *k8sRpaasManager) getIssuerMetadata(ctx context.Context, namespace, issuerName string) (map[string]string, error) { if strings.Contains(issuerName, ".") { return m.getCustomIssuerMetadata(ctx, namespace, issuerName) @@ -201,6 +236,24 @@ func findCertManagerRequestByIssuer(instance *v1alpha1.RpaasInstance, issuer str return -1, false } +func findCertManagerRequestByName(instance *v1alpha1.RpaasInstance, name string) (int, bool) { + if instance.Spec.DynamicCertificates == nil { + return -1, false + } + + if name == "" { + return -1, false + } + + for i, req := range instance.Spec.DynamicCertificates.CertManagerRequests { + if req.Name == name { + return i, true + } + } + + return -1, false +} + func areDNSNamesAllowed(allowedSuffixes, dnsNames []string) error { var unmatched []string for _, want := range dnsNames { diff --git a/internal/pkg/rpaas/certificates_test.go b/internal/pkg/rpaas/certificates_test.go index f1a0e16d..a51f8007 100644 --- a/internal/pkg/rpaas/certificates_test.go +++ b/internal/pkg/rpaas/certificates_test.go @@ -198,6 +198,33 @@ func Test_k8sRpaasManager_UpdateCertManagerRequest(t *testing.T) { }, }, + "using certificate issuer name": { + instanceName: "my-instance-1", + certManager: clientTypes.CertManager{ + Name: "cert-1", + Issuer: "issuer-1", + DNSNames: []string{"my-instance-1.example.com"}, + }, + cfg: config.RpaasConfig{ + EnableCertManager: true, + }, + assert: func(t *testing.T, cli client.Client) { + var instance v1alpha1.RpaasInstance + err := cli.Get(context.TODO(), types.NamespacedName{ + Name: "my-instance-1", + Namespace: "rpaasv2", + }, &instance) + require.NoError(t, err) + + assert.Nil(t, instance.Spec.DynamicCertificates.CertManager) + assert.Equal(t, []v1alpha1.CertManager{{ + Name: "cert-1", + Issuer: "issuer-1", + DNSNames: []string{"my-instance-1.example.com"}, + }}, instance.Spec.DynamicCertificates.CertManagerRequests) + }, + }, + "with forbidden DNS names": { instanceName: "my-instance-1", certManager: clientTypes.CertManager{ @@ -249,7 +276,7 @@ func Test_k8sRpaasManager_UpdateCertManagerRequest(t *testing.T) { } } -func Test_k8sRpaasManager_DeleteCertManagerRequest(t *testing.T) { +func Test_k8sRpaasManager_DeleteCertManagerRequestByIssuer(t *testing.T) { resources := []runtime.Object{ &v1alpha1.RpaasInstance{ ObjectMeta: metav1.ObjectMeta{ @@ -333,7 +360,7 @@ func Test_k8sRpaasManager_DeleteCertManagerRequest(t *testing.T) { manager := &k8sRpaasManager{cli: client} - err := manager.DeleteCertManagerRequest(context.TODO(), tt.instanceName, tt.issuer) + err := manager.DeleteCertManagerRequestByIssuer(context.TODO(), tt.instanceName, tt.issuer) if tt.expectedError != "" { assert.EqualError(t, err, tt.expectedError) return diff --git a/internal/pkg/rpaas/fake/manager.go b/internal/pkg/rpaas/fake/manager.go index e66a3b17..80c7a88d 100644 --- a/internal/pkg/rpaas/fake/manager.go +++ b/internal/pkg/rpaas/fake/manager.go @@ -59,10 +59,12 @@ type RpaasManager struct { FakeDeleteUpstream func(instanceName string, upstream v1alpha1.AllowedUpstream) error FakeGetCertManagerRequests func(instanceName string) ([]clientTypes.CertManager, error) FakeUpdateCertManagerRequest func(instanceName string, in clientTypes.CertManager) error - FakeDeleteCertManagerRequest func(instanceName, issuer string) error FakeGetMetadata func(instanceName string) (*clientTypes.Metadata, error) FakeSetMetadata func(instanceName string, metadata *clientTypes.Metadata) error FakeUnsetMetadata func(instanceName string, metadata *clientTypes.Metadata) error + + FakeDeleteCertManagerRequestByIssuer func(instanceName, issuer string) error + FakeDeleteCertManagerRequestByName func(instanceName, name string) error } func (m *RpaasManager) Log(ctx context.Context, instanceName string, args rpaas.LogArgs) error { @@ -346,9 +348,16 @@ func (m *RpaasManager) UpdateCertManagerRequest(ctx context.Context, instance st return nil } -func (m *RpaasManager) DeleteCertManagerRequest(ctx context.Context, instance, issuer string) error { - if m.FakeDeleteCertManagerRequest != nil { - return m.FakeDeleteCertManagerRequest(instance, issuer) +func (m *RpaasManager) DeleteCertManagerRequestByIssuer(ctx context.Context, instance, issuer string) error { + if m.FakeDeleteCertManagerRequestByIssuer != nil { + return m.FakeDeleteCertManagerRequestByIssuer(instance, issuer) + } + return nil +} + +func (m *RpaasManager) DeleteCertManagerRequestByName(ctx context.Context, instanceName, name string) error { + if m.FakeDeleteCertManagerRequestByName != nil { + return m.FakeDeleteCertManagerRequestByName(instanceName, name) } return nil } diff --git a/internal/pkg/rpaas/manager.go b/internal/pkg/rpaas/manager.go index a4e4b201..9215fef4 100644 --- a/internal/pkg/rpaas/manager.go +++ b/internal/pkg/rpaas/manager.go @@ -282,7 +282,8 @@ type RpaasManager interface { GetCertManagerRequests(ctx context.Context, instanceName string) ([]clientTypes.CertManager, error) UpdateCertManagerRequest(ctx context.Context, instanceName string, in clientTypes.CertManager) error - DeleteCertManagerRequest(ctx context.Context, instanceName, issuer string) error + DeleteCertManagerRequestByIssuer(ctx context.Context, instanceName, issuer string) error + DeleteCertManagerRequestByName(ctx context.Context, instanceName, name string) error GetMetadata(ctx context.Context, instanceName string) (*clientTypes.Metadata, error) SetMetadata(ctx context.Context, instanceName string, metadata *clientTypes.Metadata) error diff --git a/pkg/web/certificate.go b/pkg/web/certificate.go index 9e43835b..2920ee1d 100644 --- a/pkg/web/certificate.go +++ b/pkg/web/certificate.go @@ -145,8 +145,18 @@ func deleteCertManagerRequest(c echo.Context) error { return err } - if err := manager.DeleteCertManagerRequest(ctx, c.Param("instance"), c.QueryParam("issuer")); err != nil { - return err + instanceName := c.Param("instance") + name := c.QueryParam("name") + issuer := c.QueryParam("issuer") + + if name != "" { + if err := manager.DeleteCertManagerRequestByName(ctx, instanceName, name); err != nil { + return err + } + } else { + if err := manager.DeleteCertManagerRequestByIssuer(ctx, instanceName, issuer); err != nil { + return err + } } return c.NoContent(http.StatusOK) diff --git a/pkg/web/certificate_test.go b/pkg/web/certificate_test.go index 1764b497..8232b3ea 100644 --- a/pkg/web/certificate_test.go +++ b/pkg/web/certificate_test.go @@ -390,6 +390,22 @@ func Test_UpdateCertManagerRequest(t *testing.T) { expectedCode: http.StatusOK, }, + "doing a correct request with name": { + requestBody: `{"name": "cert01", "issuer": "my-issuer", "dnsNames": ["foo.example.com"]}`, + manager: &fake.RpaasManager{ + FakeUpdateCertManagerRequest: func(instanceName string, in clientTypes.CertManager) error { + assert.Equal(t, "my-instance", instanceName) + assert.Equal(t, clientTypes.CertManager{ + Name: "cert01", + Issuer: "my-issuer", + DNSNames: []string{"foo.example.com"}, + }, in) + return nil + }, + }, + expectedCode: http.StatusOK, + }, + "when some error is returned": { manager: &fake.RpaasManager{ FakeUpdateCertManagerRequest: func(instanceName string, in clientTypes.CertManager) error { @@ -417,7 +433,7 @@ func Test_UpdateCertManagerRequest(t *testing.T) { } } -func Test_DeleteCertManagerRequest(t *testing.T) { +func Test_DeleteCertManagerRequestByIssuer(t *testing.T) { tests := map[string]struct { manager rpaas.RpaasManager instance string @@ -428,7 +444,7 @@ func Test_DeleteCertManagerRequest(t *testing.T) { "remove Cert Manager request without issuer": { instance: "my-instance", manager: &fake.RpaasManager{ - FakeDeleteCertManagerRequest: func(instanceName, issuer string) error { + FakeDeleteCertManagerRequestByIssuer: func(instanceName, issuer string) error { assert.Equal(t, "my-instance", instanceName) assert.Empty(t, issuer) return nil @@ -441,7 +457,7 @@ func Test_DeleteCertManagerRequest(t *testing.T) { instance: "my-instance", issuer: "my-cert-issuer", manager: &fake.RpaasManager{ - FakeDeleteCertManagerRequest: func(instanceName, issuer string) error { + FakeDeleteCertManagerRequestByIssuer: func(instanceName, issuer string) error { assert.Equal(t, "my-instance", instanceName) assert.Equal(t, "my-cert-issuer", issuer) return nil @@ -452,7 +468,7 @@ func Test_DeleteCertManagerRequest(t *testing.T) { "when some error is returned": { manager: &fake.RpaasManager{ - FakeDeleteCertManagerRequest: func(instanceName, issuer string) error { + FakeDeleteCertManagerRequestByIssuer: func(instanceName, issuer string) error { return &rpaas.ValidationError{Msg: "some error"} }, }, @@ -476,6 +492,55 @@ func Test_DeleteCertManagerRequest(t *testing.T) { } } +func Test_DeleteCertManagerRequestByName(t *testing.T) { + tests := map[string]struct { + manager rpaas.RpaasManager + instance string + name string + expectedCode int + expectedBody string + }{ + + "remove Cert Manager request from a specific issuer": { + instance: "my-instance", + name: "cert01", + manager: &fake.RpaasManager{ + FakeDeleteCertManagerRequestByName: func(instanceName, name string) error { + assert.Equal(t, "my-instance", instanceName) + assert.Equal(t, "cert01", name) + return nil + }, + }, + expectedCode: http.StatusOK, + }, + + "when some error is returned": { + name: "cert02", + manager: &fake.RpaasManager{ + FakeDeleteCertManagerRequestByName: func(instanceName, issuer string) error { + return &rpaas.ValidationError{Msg: "some error"} + }, + }, + expectedCode: http.StatusBadRequest, + expectedBody: `{"message":"some error"}`, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + srv := newTestingServer(t, tt.manager) + defer srv.Close() + path := fmt.Sprintf("%s/resources/%s/cert-manager?name=%s", srv.URL, tt.instance, tt.name) + request, err := http.NewRequest(http.MethodDelete, path, nil) + require.NoError(t, err) + rsp, err := srv.Client().Do(request) + require.NoError(t, err) + assert.Equal(t, tt.expectedBody, bodyContent(rsp)) + assert.Equal(t, tt.expectedCode, rsp.StatusCode) + }) + } +} + func makeMultipartFormForCertificate(t *testing.T, cert, key, name string) (string, string) { b := &bytes.Buffer{} w := multipart.NewWriter(b)