Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SECURESIGN-1460 | Ensure that TSA component does not create duplicated resources #675

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions api/v1alpha1/timestampauthority_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
)

// TimestampAuthoritySpec defines the desired state of TimestampAuthority
// +kubebuilder:validation:XValidation:rule=!(has(self.signer.certificateChain.certificateChainRef) && (has(self.signer.certificateChain.intermediateCA) || has(self.signer.certificateChain.leafCA) || has(self.signer.certificateChain.rootCA))),message="when certificateChainRef is set, intermediateCA, leafCA, and rootCA must not be set"
type TimestampAuthoritySpec struct {
//Define whether you want to export service or not
ExternalAccess ExternalAccess `json:"externalAccess,omitempty"`
Expand Down Expand Up @@ -68,13 +69,13 @@ type CertificateChain struct {
CertificateChainRef *SecretKeySelector `json:"certificateChainRef,omitempty"`
//Root Certificate Authority Config
//+optional
RootCA TsaCertificateAuthority `json:"rootCA,omitempty"`
RootCA *TsaCertificateAuthority `json:"rootCA,omitempty"`
//Intermediate Certificate Authority Config
//+optional
IntermediateCA []TsaCertificateAuthority `json:"intermediateCA,omitempty"`
IntermediateCA []*TsaCertificateAuthority `json:"intermediateCA,omitempty"`
//Leaf Certificate Authority Config
//+optional
LeafCA TsaCertificateAuthority `json:"leafCA,omitempty"`
LeafCA *TsaCertificateAuthority `json:"leafCA,omitempty"`
}

// TSA Certificate Authority configuration
Expand Down
48 changes: 41 additions & 7 deletions api/v1alpha1/timestampauthority_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@ var _ = Describe("TSA", func() {
Expect(k8sClient.Get(context.Background(), getKey(created), fetched)).To(Succeed())
Expect(fetched).To(Equal(created))

fetched.Spec.Signer.CertificateChain.RootCA = TsaCertificateAuthority{
fetched.Spec.Signer.CertificateChain.RootCA = &TsaCertificateAuthority{
CommonName: "root_test1.com",
OrganizationName: "root_test1",
OrganizationEmail: "[email protected]",
}
Expect(k8sClient.Update(context.Background(), fetched)).To(Succeed())

fetched.Spec.Signer.CertificateChain.IntermediateCA[0] = TsaCertificateAuthority{
fetched.Spec.Signer.CertificateChain.IntermediateCA[0] = &TsaCertificateAuthority{
CommonName: "intermediate_test1.com",
OrganizationName: "intermediate_test1",
OrganizationEmail: "[email protected]",
}
Expect(k8sClient.Update(context.Background(), fetched)).To(Succeed())

fetched.Spec.Signer.CertificateChain.LeafCA = TsaCertificateAuthority{
fetched.Spec.Signer.CertificateChain.LeafCA = &TsaCertificateAuthority{
CommonName: "leaf_test1.com",
OrganizationName: "leaf_test1",
OrganizationEmail: "[email protected]",
Expand Down Expand Up @@ -244,8 +244,42 @@ var _ = Describe("TSA", func() {
Expect(k8sClient.Create(context.Background(), invalidObject)).
To(MatchError(ContainSubstring("only one signer should be configured at any time")))
})
})

It("intermediateCA, leafCA and rootCA cant be set if certificateChainRef is", func() {
invalidObject := generateTSAObject("invalidObj")
invalidObject.Spec.Signer = TimestampAuthoritySigner{
CertificateChain: CertificateChain{
CertificateChainRef: &SecretKeySelector{
Key: "cert_chain",
LocalObjectReference: LocalObjectReference{Name: "cert_chain"},
},
RootCA: &TsaCertificateAuthority{
CommonName: "root_test.com",
OrganizationName: "root_test",
OrganizationEmail: "[email protected]",
},
IntermediateCA: []*TsaCertificateAuthority{
{
CommonName: "intermediate_test.com",
OrganizationName: "intermediate_test",
OrganizationEmail: "[email protected]",
},
},
LeafCA: &TsaCertificateAuthority{
CommonName: "leaf_test.com",
OrganizationName: "leaf_test",
OrganizationEmail: "[email protected]",
},
},
Kms: &KMS{
KeyResource: "Key_resource",
},
}
Expect(apierrors.IsInvalid(k8sClient.Create(context.Background(), invalidObject))).To(BeTrue())
Expect(k8sClient.Create(context.Background(), invalidObject)).
To(MatchError(ContainSubstring("when certificateChainRef is set, intermediateCA, leafCA, and rootCA must not be set")))
})
})
})
})

Expand All @@ -258,19 +292,19 @@ func generateTSAObject(name string) *TimestampAuthority {
Spec: TimestampAuthoritySpec{
Signer: TimestampAuthoritySigner{
CertificateChain: CertificateChain{
RootCA: TsaCertificateAuthority{
RootCA: &TsaCertificateAuthority{
CommonName: "root_test.com",
OrganizationName: "root_test",
OrganizationEmail: "[email protected]",
},
IntermediateCA: []TsaCertificateAuthority{
IntermediateCA: []*TsaCertificateAuthority{
{
CommonName: "intermediate_test.com",
OrganizationName: "intermediate_test",
OrganizationEmail: "[email protected]",
},
},
LeafCA: TsaCertificateAuthority{
LeafCA: &TsaCertificateAuthority{
CommonName: "leaf_test.com",
OrganizationName: "leaf_test",
OrganizationEmail: "[email protected]",
Expand Down
20 changes: 16 additions & 4 deletions api/v1alpha1/zz_generated.deepcopy.go

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

6 changes: 6 additions & 0 deletions bundle/manifests/rhtas.redhat.com_securesigns.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1584,6 +1584,12 @@ spec:
required:
- signer
type: object
x-kubernetes-validations:
- message: when certificateChainRef is set, intermediateCA, leafCA,
and rootCA must not be set
rule: '!(has(self.signer.certificateChain.certificateChainRef) &&
(has(self.signer.certificateChain.intermediateCA) || has(self.signer.certificateChain.leafCA)
|| has(self.signer.certificateChain.rootCA)))'
tuf:
default:
keys:
Expand Down
5 changes: 5 additions & 0 deletions bundle/manifests/rhtas.redhat.com_timestampauthorities.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,11 @@ spec:
required:
- signer
type: object
x-kubernetes-validations:
- message: when certificateChainRef is set, intermediateCA, leafCA, and
rootCA must not be set
rule: '!(has(self.signer.certificateChain.certificateChainRef) && (has(self.signer.certificateChain.intermediateCA)
|| has(self.signer.certificateChain.leafCA) || has(self.signer.certificateChain.rootCA)))'
status:
description: TimestampAuthorityStatus defines the observed state of TimestampAuthority
properties:
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/rhtas.redhat.com_securesigns.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1584,6 +1584,12 @@ spec:
required:
- signer
type: object
x-kubernetes-validations:
- message: when certificateChainRef is set, intermediateCA, leafCA,
and rootCA must not be set
rule: '!(has(self.signer.certificateChain.certificateChainRef) &&
(has(self.signer.certificateChain.intermediateCA) || has(self.signer.certificateChain.leafCA)
|| has(self.signer.certificateChain.rootCA)))'
tuf:
default:
keys:
Expand Down
5 changes: 5 additions & 0 deletions config/crd/bases/rhtas.redhat.com_timestampauthorities.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,11 @@ spec:
required:
- signer
type: object
x-kubernetes-validations:
- message: when certificateChainRef is set, intermediateCA, leafCA, and
rootCA must not be set
rule: '!(has(self.signer.certificateChain.certificateChainRef) && (has(self.signer.certificateChain.intermediateCA)
|| has(self.signer.certificateChain.leafCA) || has(self.signer.certificateChain.rootCA)))'
status:
description: TimestampAuthorityStatus defines the observed state of TimestampAuthority
properties:
Expand Down
13 changes: 13 additions & 0 deletions internal/controller/common/action/base_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/go-logr/logr"
rhtasv1alpha1 "github.com/securesign/operator/api/v1alpha1"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/client"
client2 "sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -253,6 +254,18 @@ func EnsureAnnotations(managedAnnotations ...string) EnsureOption {
}
}

func EnsureNTPConfig() EnsureOption {
bouskaJ marked this conversation as resolved.
Show resolved Hide resolved
return func(current client.Object, expected client.Object) error {
currentTSA, ok1 := current.(*rhtasv1alpha1.TimestampAuthority)
expectedTSA, ok2 := expected.(*rhtasv1alpha1.TimestampAuthority)
if !ok1 || !ok2 {
return fmt.Errorf("EnsureNTPConfig: objects are not of type *rhtasv1alpha1.TimestampAuthority")
}
currentTSA.Spec.NTPMonitoring = expectedTSA.Spec.NTPMonitoring
return nil
}
}

func getRouteSelectorLabels(currentSpec, expectedSpec reflect.Value) (reflect.Value, reflect.Value) {
var currentRouteSelectorLabels, expectedRouteSelectorLabels reflect.Value
getRouteSelectorLabels := func(spec reflect.Value, fieldName string) reflect.Value {
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 @@ -55,7 +55,7 @@ func (i tsaAction) Handle(ctx context.Context, instance *rhtasv1alpha1.Securesig
return i.Failed(err)
}

if updated, err = i.Ensure(ctx, tsa, action.EnsureSpec(), action.EnsureRouteSelectorLabels()); err != nil {
if updated, err = i.Ensure(ctx, tsa, action.EnsureSpec(), action.EnsureRouteSelectorLabels(), action.EnsureNTPConfig()); err != nil {
meta.SetStatusCondition(&instance.Status.Conditions, v1.Condition{
Type: TSACondition,
Status: v1.ConditionFalse,
Expand Down
51 changes: 31 additions & 20 deletions internal/controller/tsa/actions/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ func (i deployAction) Name() string {

func (i deployAction) CanHandle(ctx context.Context, instance *rhtasv1alpha1.TimestampAuthority) bool {
c := meta.FindStatusCondition(instance.GetConditions(), constants.Ready)
if instance.Spec.Signer.CertificateChain.CertificateChainRef == nil &&
(instance.Spec.Signer.CertificateChain.RootCA == nil ||
instance.Spec.Signer.CertificateChain.LeafCA == nil) {
return false
}

return (c.Reason == constants.Ready || c.Reason == constants.Creating)
}

Expand All @@ -40,16 +46,18 @@ func (i deployAction) Handle(ctx context.Context, instance *rhtasv1alpha1.Timest
deployment, err := tsaUtils.CreateTimestampAuthorityDeployment(instance, DeploymentName, RBACName, labels)
if err != nil {
meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
Type: TSAServerCondition,
Status: metav1.ConditionFalse,
Reason: constants.Failure,
Message: err.Error(),
Type: TSAServerCondition,
Status: metav1.ConditionFalse,
Reason: constants.Failure,
Message: err.Error(),
ObservedGeneration: instance.Generation,
})
meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
Type: constants.Ready,
Status: metav1.ConditionFalse,
Reason: constants.Failure,
Message: err.Error(),
Type: constants.Ready,
Status: metav1.ConditionFalse,
Reason: constants.Failure,
Message: err.Error(),
ObservedGeneration: instance.Generation,
})
}
if err = controllerutil.SetControllerReference(instance, deployment, i.Client.Scheme()); err != nil {
Expand All @@ -58,26 +66,29 @@ func (i deployAction) Handle(ctx context.Context, instance *rhtasv1alpha1.Timest

if updated, err = i.Ensure(ctx, deployment); err != nil {
meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
Type: TSAServerCondition,
Status: metav1.ConditionFalse,
Reason: constants.Failure,
Message: err.Error(),
Type: TSAServerCondition,
Status: metav1.ConditionFalse,
Reason: constants.Failure,
Message: err.Error(),
ObservedGeneration: instance.Generation,
})
meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
Type: constants.Ready,
Status: metav1.ConditionFalse,
Reason: constants.Failure,
Message: err.Error(),
Type: constants.Ready,
Status: metav1.ConditionFalse,
Reason: constants.Failure,
Message: err.Error(),
ObservedGeneration: instance.Generation,
})
return i.FailedWithStatusUpdate(ctx, fmt.Errorf("could not create TSA Server: %w", err), instance)
}

if updated {
meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
Type: TSAServerCondition,
Status: metav1.ConditionFalse,
Reason: constants.Creating,
Message: "TSA server deployment created",
Type: TSAServerCondition,
Status: metav1.ConditionFalse,
Reason: constants.Creating,
Message: "TSA server deployment created",
ObservedGeneration: instance.Generation,
})
return i.StatusUpdate(ctx, instance)
} else {
Expand Down
Loading
Loading