Skip to content

Commit

Permalink
Fix deletion after upgrade
Browse files Browse the repository at this point in the history
  • Loading branch information
JasonPowr committed Sep 12, 2024
1 parent ba74073 commit 8a6e07f
Show file tree
Hide file tree
Showing 15 changed files with 64 additions and 11 deletions.
6 changes: 3 additions & 3 deletions api/v1alpha1/securesign_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ type SecuresignSpec struct {
Fulcio FulcioSpec `json:"fulcio,omitempty"`
Trillian TrillianSpec `json:"trillian,omitempty"`
//+kubebuilder:default:={keys:{{name: rekor.pub},{name: ctfe.pub},{name: fulcio_v1.crt.pem},{name: tsa.certchain.pem}}}
Tuf TufSpec `json:"tuf,omitempty"`
Ctlog CTlogSpec `json:"ctlog,omitempty"`
TimestampAuthority TimestampAuthoritySpec `json:"tsa,omitempty"`
Tuf TufSpec `json:"tuf,omitempty"`
Ctlog CTlogSpec `json:"ctlog,omitempty"`
TimestampAuthority *TimestampAuthoritySpec `json:"tsa,omitempty"`
}

// SecuresignStatus defines the observed state of Securesign
Expand Down
1 change: 1 addition & 0 deletions api/v1alpha1/timestampauthority_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type TimestampAuthoritySigner struct {
// +kubebuilder:validation:XValidation:rule="(!has(self.rootCA) && !has(self.leafCA)) || (has(self.rootCA.privateKeyRef) == has(self.leafCA.privateKeyRef))",message="must provide private keys for both root and leaf certificate authorities"
// +kubebuilder:validation:XValidation:rule=(has(self.certificateChainRef) || self.rootCA.organizationName != ""),message=organizationName cannot be empty for root certificate authority
// +kubebuilder:validation:XValidation:rule=(has(self.certificateChainRef) || self.leafCA.organizationName != ""),message=organizationName cannot be empty for leaf certificate authority
// +kubebuilder:validation:XValidation:rule=(has(self.certificateChainRef) || self.intermediateCA[0].organizationName != ""),message="organizationName cannot be empty for intermediate certificate authority, please make sure all are in place"
type CertificateChain struct {
//Reference to the certificate chain
//+optional
Expand Down
17 changes: 17 additions & 0 deletions api/v1alpha1/timestampauthority_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ var _ = Describe("TSA", func() {
Expect(k8sClient.Get(context.Background(), getKey(created), created)).ToNot(Succeed())
})

It("can be empty", func() {
created := Securesign{
Spec: SecuresignSpec{
TimestampAuthority: nil,
},
}
Expect(apierrors.IsInvalid(k8sClient.Create(context.Background(), &created))).To(BeFalse())
})

When("changing external access setting", func() {
It("enabled false->true", func() {
created := generateTSAObject("tsa-access-1")
Expand Down Expand Up @@ -126,6 +135,14 @@ var _ = Describe("TSA", func() {
To(MatchError(ContainSubstring("organizationName cannot be empty for root certificate authority")))
})

It("missing org name for intermediate CA", func() {
invalidObject := generateTSAObject("missing-org-name")
invalidObject.Spec.Signer.CertificateChain.IntermediateCA[0].OrganizationName = ""
Expect(apierrors.IsInvalid(k8sClient.Create(context.Background(), invalidObject))).To(BeTrue())
Expect(k8sClient.Create(context.Background(), invalidObject)).
To(MatchError(ContainSubstring("organizationName cannot be empty for intermediate certificate authority, please make sure all are in place")))
})

It("missing org name for leaf CA", func() {
invalidObject := generateTSAObject("missing-org-name")
invalidObject.Spec.Signer.CertificateChain.LeafCA.OrganizationName = ""
Expand Down
6 changes: 5 additions & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions bundle/manifests/rhtas.redhat.com_securesigns.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1161,6 +1161,10 @@ spec:
authority
rule: (has(self.certificateChainRef) || self.leafCA.organizationName
!= "")
- message: organizationName cannot be empty for intermediate
certificate authority, please make sure all are in place
rule: (has(self.certificateChainRef) || self.intermediateCA[0].organizationName
!= "")
file:
description: Configuration for file-based signer
properties:
Expand Down
8 changes: 8 additions & 0 deletions bundle/manifests/rhtas.redhat.com_timestampauthorities.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,10 @@ spec:
authority
rule: (has(self.certificateChainRef) || self.leafCA.organizationName
!= "")
- message: organizationName cannot be empty for intermediate certificate
authority, please make sure all are in place
rule: (has(self.certificateChainRef) || self.intermediateCA[0].organizationName
!= "")
file:
description: Configuration for file-based signer
properties:
Expand Down Expand Up @@ -1075,6 +1079,10 @@ spec:
authority
rule: (has(self.certificateChainRef) || self.leafCA.organizationName
!= "")
- message: organizationName cannot be empty for intermediate certificate
authority, please make sure all are in place
rule: (has(self.certificateChainRef) || self.intermediateCA[0].organizationName
!= "")
file:
description: Configuration for file-based signer
properties:
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/rhtas.redhat.com_securesigns.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1161,6 +1161,10 @@ spec:
authority
rule: (has(self.certificateChainRef) || self.leafCA.organizationName
!= "")
- message: organizationName cannot be empty for intermediate
certificate authority, please make sure all are in place
rule: (has(self.certificateChainRef) || self.intermediateCA[0].organizationName
!= "")
file:
description: Configuration for file-based signer
properties:
Expand Down
8 changes: 8 additions & 0 deletions config/crd/bases/rhtas.redhat.com_timestampauthorities.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,10 @@ spec:
authority
rule: (has(self.certificateChainRef) || self.leafCA.organizationName
!= "")
- message: organizationName cannot be empty for intermediate certificate
authority, please make sure all are in place
rule: (has(self.certificateChainRef) || self.intermediateCA[0].organizationName
!= "")
file:
description: Configuration for file-based signer
properties:
Expand Down Expand Up @@ -1075,6 +1079,10 @@ spec:
authority
rule: (has(self.certificateChainRef) || self.leafCA.organizationName
!= "")
- message: organizationName cannot be empty for intermediate certificate
authority, please make sure all are in place
rule: (has(self.certificateChainRef) || self.intermediateCA[0].organizationName
!= "")
file:
description: Configuration for file-based signer
properties:
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/securesign/actions/ensure_tsa.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (i tsaAction) Handle(ctx context.Context, instance *rhtasv1alpha1.Securesig
})
return i.StatusUpdate(ctx, instance)
}
tsa.Spec = instance.Spec.TimestampAuthority
tsa.Spec = *instance.Spec.TimestampAuthority

if err = controllerutil.SetControllerReference(instance, tsa, i.Client.Scheme()); err != nil {
return i.Failed(err)
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/byodb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ var _ = Describe("Securesign install with byodb", Ordered, func() {
Name: "my-db",
},
}},
TimestampAuthority: v1alpha1.TimestampAuthoritySpec{
TimestampAuthority: &v1alpha1.TimestampAuthoritySpec{
ExternalAccess: v1alpha1.ExternalAccess{
Enabled: true,
},
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/common_install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ var _ = Describe("Securesign install with certificate generation", Ordered, func
Trillian: v1alpha1.TrillianSpec{Db: v1alpha1.TrillianDB{
Create: ptr.To(true),
}},
TimestampAuthority: v1alpha1.TimestampAuthoritySpec{
TimestampAuthority: &v1alpha1.TimestampAuthoritySpec{
ExternalAccess: v1alpha1.ExternalAccess{
Enabled: true,
},
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/key_autodiscovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ var _ = Describe("Securesign key autodiscovery test", Ordered, func() {
Retain: ptr.To(false),
},
}},
TimestampAuthority: v1alpha1.TimestampAuthoritySpec{
TimestampAuthority: &v1alpha1.TimestampAuthoritySpec{
ExternalAccess: v1alpha1.ExternalAccess{
Enabled: true,
},
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/provided_certs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ var _ = Describe("Securesign install with provided certs", Ordered, func() {
Retain: ptr.To(false),
},
}},
TimestampAuthority: v1alpha1.TimestampAuthoritySpec{
TimestampAuthority: &v1alpha1.TimestampAuthoritySpec{
ExternalAccess: v1alpha1.ExternalAccess{
Enabled: true,
},
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/update/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func securesignResource(namespace *v1.Namespace) *rhtasv1alpha1.Securesign {
Retain: ptr.To(false),
},
}},
TimestampAuthority: rhtasv1alpha1.TimestampAuthoritySpec{
TimestampAuthority: &rhtasv1alpha1.TimestampAuthoritySpec{
ExternalAccess: rhtasv1alpha1.ExternalAccess{
Enabled: true,
},
Expand Down
9 changes: 8 additions & 1 deletion test/e2e/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ var _ = Describe("Operator upgrade", Ordered, func() {
gomega.Expect(s).ToNot(gomega.BeNil())
gomega.Expect(meta.FindStatusCondition(s.GetConditions(), actions.TSACondition).Reason).To(gomega.Equal(constants.NotDefined))

s.Spec.TimestampAuthority = tasv1alpha.TimestampAuthoritySpec{
s.Spec.TimestampAuthority = &tasv1alpha.TimestampAuthoritySpec{
ExternalAccess: tasv1alpha.ExternalAccess{
Enabled: true,
},
Expand Down Expand Up @@ -555,6 +555,13 @@ var _ = Describe("Operator upgrade", Ordered, func() {
newImageName,
)).To(gomega.Succeed())
})

It("Make sure securesign can be deleted after upgrade", func() {
gomega.Eventually(func(g gomega.Gomega) {
s := securesign.Get(ctx, cli, namespace.Name, securesignDeployment.Name)()
gomega.Expect(cli.Delete(ctx, s)).Should(gomega.Succeed())
}).Should(gomega.Succeed())
})
})

func findClusterServiceVersion(ctx context.Context, cli runtimeCli.Client, conditions func(version v1alpha1.ClusterServiceVersion) bool, ns string) func() *v1alpha1.ClusterServiceVersion {
Expand Down

0 comments on commit 8a6e07f

Please sign in to comment.