Skip to content

Commit

Permalink
Add support to delete and update by name
Browse files Browse the repository at this point in the history
  • Loading branch information
wpjunior committed Aug 12, 2024
1 parent e6f47fc commit e66688c
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 15 deletions.
57 changes: 55 additions & 2 deletions internal/pkg/rpaas/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
31 changes: 29 additions & 2 deletions internal/pkg/rpaas/certificates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down
17 changes: 13 additions & 4 deletions internal/pkg/rpaas/fake/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion internal/pkg/rpaas/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions pkg/web/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
73 changes: 69 additions & 4 deletions pkg/web/certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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"}
},
},
Expand All @@ -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)
Expand Down

0 comments on commit e66688c

Please sign in to comment.