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

Use metav1.Condition instead of cmapi.IssuerCondition in Issuer API #187

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
3 changes: 2 additions & 1 deletion api/v1alpha1/issuer_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import (
type Issuer interface {
runtime.Object
metav1.Object
GetStatus() *IssuerStatus

GetConditions() []metav1.Condition

// GetIssuerTypeIdentifier returns a string that uniquely identifies the
// issuer type. This should be a constant across all instances of this
Expand Down
6 changes: 2 additions & 4 deletions api/v1alpha1/issuer_status_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ limitations under the License.

package v1alpha1

import (
cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
)
import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

type IssuerStatus struct {
// List of status conditions to indicate the status of an Issuer.
// Known condition types are `Ready`.
// +listType=map
// +listMapKey=type
// +optional
Conditions []cmapi.IssuerCondition `json:"conditions,omitempty"`
Conditions []metav1.Condition `json:"conditions,omitempty"`
}
6 changes: 6 additions & 0 deletions api/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ limitations under the License.

package v1alpha1

const (
// IssuerConditionTypeReady is the type of condition that indicates whether
// an Issuer is ready for use.
IssuerConditionTypeReady = "Ready"
)

const (
// CertificateRequestConditionReasonInitializing is the value assigned to
// the Reason field of the Ready condition when issuer-lib first
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/zz_generated.deepcopy.go

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

26 changes: 12 additions & 14 deletions conditions/issuer.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ limitations under the License.
package conditions

import (
cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/clock"
)
Expand All @@ -27,14 +25,14 @@ import (
// the added condition.
func SetIssuerStatusCondition(
clock clock.PassiveClock,
existingConditions []cmapi.IssuerCondition,
patchConditions *[]cmapi.IssuerCondition,
existingConditions []metav1.Condition,
patchConditions *[]metav1.Condition,
observedGeneration int64,
conditionType cmapi.IssuerConditionType,
status cmmeta.ConditionStatus,
conditionType string,
status metav1.ConditionStatus,
reason, message string,
) (*cmapi.IssuerCondition, *metav1.Time) {
newCondition := cmapi.IssuerCondition{
) (*metav1.Condition, metav1.Time) {
newCondition := metav1.Condition{
Type: conditionType,
Status: status,
Reason: reason,
Expand All @@ -43,7 +41,7 @@ func SetIssuerStatusCondition(
}

nowTime := metav1.NewTime(clock.Now())
newCondition.LastTransitionTime = &nowTime
newCondition.LastTransitionTime = nowTime

// Reset the LastTransitionTime if the status hasn't changed
for _, cond := range existingConditions {
Expand All @@ -68,20 +66,20 @@ func SetIssuerStatusCondition(
// Overwrite the existing condition
(*patchConditions)[idx] = newCondition

return &newCondition, &nowTime
return &newCondition, nowTime
}

// If we've not found an existing condition of this type, we simply insert
// the new condition into the slice.
*patchConditions = append(*patchConditions, newCondition)

return &newCondition, &nowTime
return &newCondition, nowTime
}

func GetIssuerStatusCondition(
conditions []cmapi.IssuerCondition,
conditionType cmapi.IssuerConditionType,
) *cmapi.IssuerCondition {
conditions []metav1.Condition,
conditionType string,
) *metav1.Condition {
for _, cond := range conditions {
if cond.Type == conditionType {
return &cond
Expand Down
114 changes: 57 additions & 57 deletions conditions/issuer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,23 @@ package conditions
import (
"testing"

cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clocktesting "k8s.io/utils/clock/testing"

"github.com/cert-manager/issuer-lib/api/v1alpha1"
)

func TestSetIssuerStatusCondition(t *testing.T) {
type testCase struct {
name string

existingConditions []cmapi.IssuerCondition
patchConditions []cmapi.IssuerCondition
conditionType cmapi.IssuerConditionType
status cmmeta.ConditionStatus
existingConditions []metav1.Condition
patchConditions []metav1.Condition
conditionType string
status metav1.ConditionStatus

expectedCondition cmapi.IssuerCondition
expectedCondition metav1.Condition
expectNewEntry bool
}

Expand All @@ -49,93 +49,93 @@ func TestSetIssuerStatusCondition(t *testing.T) {
testCases := []testCase{
{
name: "if the condition does NOT change its status, the last transition time should not be updated",
existingConditions: []cmapi.IssuerCondition{
existingConditions: []metav1.Condition{
{
Type: cmapi.IssuerConditionReady,
Status: cmmeta.ConditionTrue,
Type: v1alpha1.IssuerConditionTypeReady,
Status: metav1.ConditionTrue,
},
},
patchConditions: []cmapi.IssuerCondition{},
conditionType: cmapi.IssuerConditionReady,
status: cmmeta.ConditionTrue,

expectedCondition: cmapi.IssuerCondition{
Type: cmapi.IssuerConditionReady,
Status: cmmeta.ConditionTrue,
LastTransitionTime: &fakeTimeObj1,
patchConditions: []metav1.Condition{},
conditionType: v1alpha1.IssuerConditionTypeReady,
status: metav1.ConditionTrue,

expectedCondition: metav1.Condition{
Type: v1alpha1.IssuerConditionTypeReady,
Status: metav1.ConditionTrue,
LastTransitionTime: fakeTimeObj1,
},
expectNewEntry: true,
},
{
name: "if the condition DOES change its status, the last transition time should be updated",
existingConditions: []cmapi.IssuerCondition{
existingConditions: []metav1.Condition{
{
Type: cmapi.IssuerConditionReady,
Status: cmmeta.ConditionTrue,
Type: v1alpha1.IssuerConditionTypeReady,
Status: metav1.ConditionTrue,
},
},
patchConditions: []cmapi.IssuerCondition{},
conditionType: cmapi.IssuerConditionReady,
status: cmmeta.ConditionFalse,

expectedCondition: cmapi.IssuerCondition{
Type: cmapi.IssuerConditionReady,
Status: cmmeta.ConditionFalse,
LastTransitionTime: &fakeTimeObj2,
patchConditions: []metav1.Condition{},
conditionType: v1alpha1.IssuerConditionTypeReady,
status: metav1.ConditionFalse,

expectedCondition: metav1.Condition{
Type: v1alpha1.IssuerConditionTypeReady,
Status: metav1.ConditionFalse,
LastTransitionTime: fakeTimeObj2,
},
expectNewEntry: true,
},
{
name: "if the patch contains already contains the condition, it should get overwritten",
existingConditions: []cmapi.IssuerCondition{
existingConditions: []metav1.Condition{
{
Type: cmapi.IssuerConditionReady,
Status: cmmeta.ConditionTrue,
Type: v1alpha1.IssuerConditionTypeReady,
Status: metav1.ConditionTrue,
},
},
patchConditions: []cmapi.IssuerCondition{
patchConditions: []metav1.Condition{
{
Type: cmapi.IssuerConditionReady,
Status: cmmeta.ConditionTrue,
Type: v1alpha1.IssuerConditionTypeReady,
Status: metav1.ConditionTrue,
},
},
conditionType: cmapi.IssuerConditionReady,
status: cmmeta.ConditionTrue,
conditionType: v1alpha1.IssuerConditionTypeReady,
status: metav1.ConditionTrue,

expectedCondition: cmapi.IssuerCondition{
Type: cmapi.IssuerConditionReady,
Status: cmmeta.ConditionTrue,
LastTransitionTime: &fakeTimeObj1,
expectedCondition: metav1.Condition{
Type: v1alpha1.IssuerConditionTypeReady,
Status: metav1.ConditionTrue,
LastTransitionTime: fakeTimeObj1,
},
expectNewEntry: false,
},
{
name: "if the patch contains another condition type, it should get added",
existingConditions: []cmapi.IssuerCondition{
existingConditions: []metav1.Condition{
{
Type: cmapi.IssuerConditionReady,
Status: cmmeta.ConditionTrue,
Type: v1alpha1.IssuerConditionTypeReady,
Status: metav1.ConditionTrue,
},
},
patchConditions: []cmapi.IssuerCondition{
patchConditions: []metav1.Condition{
{
Type: cmapi.IssuerConditionReady,
Status: cmmeta.ConditionTrue,
Type: v1alpha1.IssuerConditionTypeReady,
Status: metav1.ConditionTrue,
},
},
conditionType: cmapi.IssuerConditionType("AnotherCondition"),
status: cmmeta.ConditionTrue,
conditionType: "AnotherCondition",
status: metav1.ConditionTrue,

expectedCondition: cmapi.IssuerCondition{
Type: cmapi.IssuerConditionType("AnotherCondition"),
Status: cmmeta.ConditionTrue,
LastTransitionTime: &fakeTimeObj2,
expectedCondition: metav1.Condition{
Type: "AnotherCondition",
Status: metav1.ConditionTrue,
LastTransitionTime: fakeTimeObj2,
},
expectNewEntry: true,
},
}

defaultConditions := func(t *testing.T, conditions []cmapi.IssuerCondition) []cmapi.IssuerCondition {
defaultConditions := func(t *testing.T, conditions []metav1.Condition) []metav1.Condition {
t.Helper()

for i := range conditions {
Expand All @@ -145,7 +145,7 @@ func TestSetIssuerStatusCondition(t *testing.T) {
conditions[i].ObservedGeneration != 0 {
t.Fatal("this field is managed by the test and should not be set")
}
conditions[i].LastTransitionTime = &fakeTimeObj1
conditions[i].LastTransitionTime = fakeTimeObj1
conditions[i].Reason = "OldReason"
conditions[i].Message = "OldMessage"
conditions[i].ObservedGeneration = 7
Expand All @@ -159,7 +159,7 @@ func TestSetIssuerStatusCondition(t *testing.T) {
test.existingConditions = defaultConditions(t, test.existingConditions)
test.patchConditions = defaultConditions(t, test.patchConditions)

patchConditions := append([]cmapi.IssuerCondition{}, test.patchConditions...)
patchConditions := append([]metav1.Condition{}, test.patchConditions...)

cond, time := SetIssuerStatusCondition(
fakeClock2,
Expand All @@ -181,7 +181,7 @@ func TestSetIssuerStatusCondition(t *testing.T) {
test.expectedCondition.Message = "NewMessage"
test.expectedCondition.ObservedGeneration = 8
require.Equal(t, test.expectedCondition, *cond)
require.Equal(t, &fakeTimeObj2, time)
require.Equal(t, fakeTimeObj2, time)

// Check that the patchConditions slice got a new entry if expected
if test.expectNewEntry {
Expand Down
4 changes: 2 additions & 2 deletions controllers/certificaterequest_controller_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,8 @@ func markIssuerReady(t *testing.T, ctx context.Context, kc client.Client, clock
issuerStatus.Conditions,
&issuerStatus.Conditions,
issuer.GetGeneration(),
cmapi.IssuerConditionReady,
cmmeta.ConditionTrue,
v1alpha1.IssuerConditionTypeReady,
metav1.ConditionTrue,
v1alpha1.IssuerConditionReasonChecked,
"Succeeded checking the issuer",
)
Expand Down
12 changes: 6 additions & 6 deletions controllers/certificaterequest_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) {
testutil.SetTestIssuerGeneration(70),
testutil.SetTestIssuerStatusCondition(
fakeClock1,
cmapi.IssuerConditionReady,
cmmeta.ConditionTrue,
v1alpha1.IssuerConditionTypeReady,
metav1.ConditionTrue,
v1alpha1.IssuerConditionReasonChecked,
"Succeeded checking the issuer",
),
Expand All @@ -91,8 +91,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) {
testutil.SetTestClusterIssuerGeneration(70),
testutil.SetTestClusterIssuerStatusCondition(
fakeClock1,
cmapi.IssuerConditionReady,
cmmeta.ConditionTrue,
v1alpha1.IssuerConditionTypeReady,
metav1.ConditionTrue,
v1alpha1.IssuerConditionReasonChecked,
"Succeeded checking the issuer",
),
Expand Down Expand Up @@ -335,8 +335,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) {
testutil.TestIssuerFrom(issuer1,
testutil.SetTestIssuerStatusCondition(
fakeClock1,
cmapi.IssuerConditionReady,
cmmeta.ConditionFalse,
v1alpha1.IssuerConditionTypeReady,
metav1.ConditionFalse,
"[REASON]",
"[MESSAGE]",
),
Expand Down
12 changes: 6 additions & 6 deletions controllers/certificatesigningrequest_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) {
testutil.SetTestIssuerGeneration(70),
testutil.SetTestIssuerStatusCondition(
fakeClock1,
cmapi.IssuerConditionReady,
cmmeta.ConditionTrue,
v1alpha1.IssuerConditionTypeReady,
metav1.ConditionTrue,
v1alpha1.IssuerConditionReasonChecked,
"Succeeded checking the issuer",
),
Expand All @@ -93,8 +93,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) {
testutil.SetTestClusterIssuerGeneration(70),
testutil.SetTestClusterIssuerStatusCondition(
fakeClock1,
cmapi.IssuerConditionReady,
cmmeta.ConditionTrue,
v1alpha1.IssuerConditionTypeReady,
metav1.ConditionTrue,
v1alpha1.IssuerConditionReasonChecked,
"Succeeded checking the issuer",
),
Expand Down Expand Up @@ -250,8 +250,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) {
testutil.TestClusterIssuerFrom(clusterIssuer1,
testutil.SetTestClusterIssuerStatusCondition(
fakeClock1,
cmapi.IssuerConditionReady,
cmmeta.ConditionFalse,
v1alpha1.IssuerConditionTypeReady,
metav1.ConditionFalse,
"[REASON]",
"[MESSAGE]",
),
Expand Down
Loading