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.

Sign up for GitHub

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

Jump to bottom

[release-v1.34] [Bugfix] wrong interpretation of custom cert rotation params #1792

Merged
merged 1 commit into from
May 19, 2021
Merged

[release-v1.34] [Bugfix] wrong interpretation of custom cert rotation params #1792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions pkg/operator/controller/certrotation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,11 @@ var _ = Describe("Cert rotation tests", func() {
n := time.Now()

args := &cert.FactoryArgs{
Namespace: namespace,
SignerValidity: pt(50 * time.Hour),
SignerRefresh: pt(25 * time.Hour),
TargetValidity: pt(26 * time.Hour),
TargetRefresh: pt(13 * time.Hour),
Namespace: namespace,
SignerDuration: pt(50 * time.Hour),
SignerRenewBefore: pt(25 * time.Hour),
TargetDuration: pt(26 * time.Hour),
TargetRenewBefore: pt(13 * time.Hour),
}

certs = cert.CreateCertificateDefinitions(args)
Expand Down
8 changes: 4 additions & 4 deletions pkg/operator/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,21 +190,21 @@ func (r *ReconcileCDI) getCertificateDefinitions(cdi *cdiv1.CDI) []cdicerts.Cert
if cdi != nil && cdi.Spec.CertConfig != nil {
if cdi.Spec.CertConfig.CA != nil {
if cdi.Spec.CertConfig.CA.Duration != nil {
args.SignerValidity = &cdi.Spec.CertConfig.CA.Duration.Duration
args.SignerDuration = &cdi.Spec.CertConfig.CA.Duration.Duration
}

if cdi.Spec.CertConfig.CA.RenewBefore != nil {
args.SignerRefresh = &cdi.Spec.CertConfig.CA.RenewBefore.Duration
args.SignerRenewBefore = &cdi.Spec.CertConfig.CA.RenewBefore.Duration
}
}

if cdi.Spec.CertConfig.Server != nil {
if cdi.Spec.CertConfig.Server.Duration != nil {
args.TargetValidity = &cdi.Spec.CertConfig.Server.Duration.Duration
args.TargetDuration = &cdi.Spec.CertConfig.Server.Duration.Duration
}

if cdi.Spec.CertConfig.Server.RenewBefore != nil {
args.TargetRefresh = &cdi.Spec.CertConfig.Server.RenewBefore.Duration
args.TargetRenewBefore = &cdi.Spec.CertConfig.Server.RenewBefore.Duration
}
}
}
Expand Down
57 changes: 15 additions & 42 deletions pkg/operator/controller/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ var _ = Describe("Controller", func() {
func(toModify runtime.Object) (runtime.Object, runtime.Object, error) { //Modify
deploymentOrig, ok := toModify.(*appsv1.Deployment)
if !ok {
return toModify, toModify, generrors.New(fmt.Sprint("wrong type"))
return toModify, toModify, generrors.New("wrong type")
}
deployment := deploymentOrig.DeepCopy()
deployment.Annotations["fake.anno.1"] = "fakeannotation1"
Expand Down Expand Up @@ -815,17 +815,13 @@ var _ = Describe("Controller", func() {
}
}

if len(desiredDep.Annotations) > len(postDep.Annotations) {
return false
}

return true
return len(desiredDep.Annotations) <= len(postDep.Annotations)
}),
Entry("verify - deployment updated on upgrade - labels changed",
func(toModify runtime.Object) (runtime.Object, runtime.Object, error) { //Modify
deploymentOrig, ok := toModify.(*appsv1.Deployment)
if !ok {
return toModify, toModify, generrors.New(fmt.Sprint("wrong type"))
return toModify, toModify, generrors.New("wrong type")
}
deployment := deploymentOrig.DeepCopy()
deployment.Labels["fake.label.1"] = "fakelabel1"
Expand Down Expand Up @@ -856,17 +852,13 @@ var _ = Describe("Controller", func() {
}
}

if len(desiredDep.Labels) > len(postDep.Labels) {
return false
}

return true
return len(desiredDep.Labels) <= len(postDep.Labels)
}),
Entry("verify - deployment updated on upgrade - deployment spec changed - modify container",
func(toModify runtime.Object) (runtime.Object, runtime.Object, error) { //Modify
deploymentOrig, ok := toModify.(*appsv1.Deployment)
if !ok {
return toModify, toModify, generrors.New(fmt.Sprint("wrong type"))
return toModify, toModify, generrors.New("wrong type")
}
deployment := deploymentOrig.DeepCopy()

Expand Down Expand Up @@ -909,17 +901,13 @@ var _ = Describe("Controller", func() {
}
}

if len(desiredDep.Spec.Template.Spec.Containers[0].Env) != len(postDep.Spec.Template.Spec.Containers[0].Env) {
return false
}

return true
return len(desiredDep.Spec.Template.Spec.Containers[0].Env) == len(postDep.Spec.Template.Spec.Containers[0].Env)
}),
Entry("verify - deployment updated on upgrade - deployment spec changed - add new container",
func(toModify runtime.Object) (runtime.Object, runtime.Object, error) { //Modify
deploymentOrig, ok := toModify.(*appsv1.Deployment)
if !ok {
return toModify, toModify, generrors.New(fmt.Sprint("wrong type"))
return toModify, toModify, generrors.New("wrong type")
}
deployment := deploymentOrig.DeepCopy()

Expand Down Expand Up @@ -963,17 +951,13 @@ var _ = Describe("Controller", func() {
}
}

if len(desiredDep.Spec.Template.Spec.Containers) > len(postDep.Spec.Template.Spec.Containers) {
return false
}

return true
return len(desiredDep.Spec.Template.Spec.Containers) <= len(postDep.Spec.Template.Spec.Containers)
}),
Entry("verify - deployment updated on upgrade - deployment spec changed - remove existing container",
func(toModify runtime.Object) (runtime.Object, runtime.Object, error) { //Modify
deploymentOrig, ok := toModify.(*appsv1.Deployment)
if !ok {
return toModify, toModify, generrors.New(fmt.Sprint("wrong type"))
return toModify, toModify, generrors.New("wrong type")
}
deployment := deploymentOrig.DeepCopy()

Expand Down Expand Up @@ -1012,7 +996,7 @@ var _ = Describe("Controller", func() {
func(toModify runtime.Object) (runtime.Object, runtime.Object, error) { //Modify
serviceOrig, ok := toModify.(*corev1.Service)
if !ok {
return toModify, toModify, generrors.New(fmt.Sprint("wrong type"))
return toModify, toModify, generrors.New("wrong type")
}
service := serviceOrig.DeepCopy()
service.Annotations["fake.anno.1"] = "fakeannotation1"
Expand Down Expand Up @@ -1043,17 +1027,14 @@ var _ = Describe("Controller", func() {
}
}

if len(desired.Annotations) > len(post.Annotations) {
return false
}
return true
return len(desired.Annotations) <= len(post.Annotations)
}),

Entry("verify - services updated on upgrade - label changed",
func(toModify runtime.Object) (runtime.Object, runtime.Object, error) { //Modify
serviceOrig, ok := toModify.(*corev1.Service)
if !ok {
return toModify, toModify, generrors.New(fmt.Sprint("wrong type"))
return toModify, toModify, generrors.New("wrong type")
}
service := serviceOrig.DeepCopy()
service.Labels["fake.label.1"] = "fakelabel1"
Expand Down Expand Up @@ -1084,18 +1065,14 @@ var _ = Describe("Controller", func() {
}
}

if len(desired.Labels) > len(post.Labels) {
return false
}

return true
return len(desired.Labels) <= len(post.Labels)
}),

Entry("verify - services updated on upgrade - service port changed",
func(toModify runtime.Object) (runtime.Object, runtime.Object, error) { //Modify
serviceOrig, ok := toModify.(*corev1.Service)
if !ok {
return toModify, toModify, generrors.New(fmt.Sprint("wrong type"))
return toModify, toModify, generrors.New("wrong type")
}
service := serviceOrig.DeepCopy()
service.Spec.Ports = []corev1.ServicePort{
Expand Down Expand Up @@ -1129,11 +1106,7 @@ var _ = Describe("Controller", func() {
}
}

if len(desired.Spec.Ports) != len(post.Spec.Ports) {
return false
}

return true
return len(desired.Spec.Ports) == len(post.Spec.Ports)
}),
//CRD update
// - update CRD label
Expand Down
28 changes: 16 additions & 12 deletions pkg/operator/resources/cert/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ const day = 24 * time.Hour
type FactoryArgs struct {
Namespace string

SignerValidity *time.Duration
SignerRefresh *time.Duration
SignerDuration *time.Duration
// Duration to subtract from cert NotAfter value
SignerRenewBefore *time.Duration

TargetValidity *time.Duration
TargetRefresh *time.Duration
TargetDuration *time.Duration
// Duration to subtract from cert NotAfter value
TargetRenewBefore *time.Duration
}

// CertificateConfig contains cert configuration data
Expand Down Expand Up @@ -86,20 +88,22 @@ func CreateCertificateDefinitions(args *FactoryArgs) []CertificateDefinition {
}

if def.Configurable {
if args.SignerValidity != nil {
def.SignerConfig.Lifetime = *args.SignerValidity
if args.SignerDuration != nil {
def.SignerConfig.Lifetime = *args.SignerDuration
}

if args.SignerRefresh != nil {
def.SignerConfig.Refresh = *args.SignerRefresh
if args.SignerRenewBefore != nil {
// convert to time from cert NotBefore
def.SignerConfig.Refresh = def.SignerConfig.Lifetime - *args.SignerRenewBefore
}

if args.TargetValidity != nil {
def.TargetConfig.Lifetime = *args.TargetValidity
if args.TargetDuration != nil {
def.TargetConfig.Lifetime = *args.TargetDuration
}

if args.TargetRefresh != nil {
def.TargetConfig.Refresh = *args.TargetRefresh
if args.TargetRenewBefore != nil {
// convert to time from cert NotBefore
def.TargetConfig.Refresh = def.TargetConfig.Lifetime - *args.TargetRenewBefore
}
}
}
Expand Down
14 changes: 7 additions & 7 deletions tests/certrotation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import (
)

const (
notAfter = "auth.openshift.io/certificate-not-after"
notBefore = "auth.openshift.io/certificate-not-before"
annNotAfter = "auth.openshift.io/certificate-not-after"
annNotBefore = "auth.openshift.io/certificate-not-before"
)

var _ = Describe("Cert rotation tests", func() {
Expand Down Expand Up @@ -88,7 +88,7 @@ var _ = Describe("Cert rotation tests", func() {
updatedSecret, err := f.K8sClient.CoreV1().Secrets(f.CdiInstallNs).Get(context.TODO(), secretName, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())

return updatedSecret.Annotations[notAfter] != oldSecret.Annotations[notAfter] &&
return updatedSecret.Annotations[annNotAfter] != oldSecret.Annotations[annNotAfter] &&
string(updatedSecret.Data["tls.cert"]) != string(oldSecret.Data["tls.crt"]) &&
string(updatedSecret.Data["tls.key"]) != string(oldSecret.Data["tls.key"])

Expand Down Expand Up @@ -121,17 +121,17 @@ func rotateCert(f *framework.Framework, secretName string) {
secret, err := f.K8sClient.CoreV1().Secrets(f.CdiInstallNs).Get(context.TODO(), secretName, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())

nb, ok := secret.Annotations[notBefore]
nb, ok := secret.Annotations[annNotBefore]
Expect(ok).To(BeTrue())

notBefore, err := time.Parse(time.RFC3339, nb)
Expect(err).ToNot(HaveOccurred())
Expect(time.Now().Sub(notBefore).Seconds() > 0).To(BeTrue())
Expect(time.Since(notBefore).Seconds() > 0).To(BeTrue())

newSecret := secret.DeepCopy()
newSecret.Annotations[notAfter] = notBefore.Add(time.Second).Format(time.RFC3339)
newSecret.Annotations[annNotAfter] = notBefore.Add(time.Second).Format(time.RFC3339)

newSecret, err = f.K8sClient.CoreV1().Secrets(f.CdiInstallNs).Update(context.TODO(), newSecret, metav1.UpdateOptions{})
_, err = f.K8sClient.CoreV1().Secrets(f.CdiInstallNs).Update(context.TODO(), newSecret, metav1.UpdateOptions{})
Expect(err).ToNot(HaveOccurred())
}

Expand Down