Skip to content

Commit 7c1faf7

Browse files
isometryisometry
committed
refactor: Remove unused methods and clean up CRD scaffolding
- Remove unused GetName methods from Token and ClusterToken types - Remove HasStartupConfig from ghapp.Registry and related test - Remove commented and scaffolding code from CRDs and API types - Refactor tokenmanager to simplify tokenSecret construction and context usage - Update controller and tokenmanager to use new tokenSecret API
1 parent cb9ffb8 commit 7c1faf7

10 files changed

Lines changed: 106 additions & 222 deletions

File tree

‎api/v1/clustertoken_types.go‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,8 @@ import (
2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2626
)
2727

28-
// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
29-
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.
30-
3128
// ClusterTokenSpec defines the desired state of ClusterToken
3229
type ClusterTokenSpec struct {
33-
// Important: Run "make" to regenerate code after modifying this file
34-
3530
// +optional
3631
// Reference to the App that provides the GitHub App credentials for this
3732
// ClusterToken. When spec.appRef.namespace is empty, the operator resolves
@@ -104,8 +99,6 @@ type ClusterTokenSecretSpec struct {
10499

105100
// ClusterTokenStatus defines the observed state of ClusterToken
106101
type ClusterTokenStatus struct {
107-
// Important: Run "make" to regenerate code after modifying this file
108-
109102
ManagedSecret ManagedSecret `json:"managedSecret,omitempty"`
110103

111104
IAT InstallationAccessToken `json:"installationAccessToken,omitempty"`
@@ -130,10 +123,6 @@ func (t *ClusterToken) GetType() string {
130123
return "ClusterToken"
131124
}
132125

133-
func (t *ClusterToken) GetName() string {
134-
return t.Name
135-
}
136-
137126
func (t *ClusterToken) GetInstallationID() int64 {
138127
return t.Spec.InstallationID
139128
}

‎api/v1/managed_secret.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ func (m ManagedSecret) MatchesSpec(owner secretOwner) bool {
2222
return m.Namespace == owner.GetSecretNamespace() && m.Name == owner.GetSecretName() && m.BasicAuth == owner.GetSecretBasicAuth()
2323
}
2424

25-
// func (m *ManagedSecret) MatchesKey(key types.NamespacedName) bool {
26-
// return m.Namespace == key.Namespace && m.Name == key.Name
27-
// }
28-
2925
func (m ManagedSecret) Key() types.NamespacedName {
3026
return types.NamespacedName{
3127
Namespace: m.Namespace,

‎api/v1/token_types.go‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,8 @@ import (
2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2626
)
2727

28-
// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
29-
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.
30-
3128
// TokenSpec defines the desired state of Token
3229
type TokenSpec struct {
33-
// Important: Run "make" to regenerate code after modifying this file
34-
3530
// +optional
3631
// Reference to the App that provides the GitHub App credentials for this
3732
// Token. Must be in the same namespace as the Token. When unset, the
@@ -98,7 +93,6 @@ type TokenSecretSpec struct {
9893

9994
// TokenStatus defines the observed state of Token
10095
type TokenStatus struct {
101-
// Important: Run "make" to regenerate code after modifying this file
10296
ManagedSecret ManagedSecret `json:"managedSecret,omitempty"`
10397

10498
IAT InstallationAccessToken `json:"installationAccessToken,omitempty"`
@@ -122,10 +116,6 @@ func (t *Token) GetType() string {
122116
return "Token"
123117
}
124118

125-
func (t *Token) GetName() string {
126-
return t.Name
127-
}
128-
129119
func (t *Token) GetInstallationID() int64 {
130120
return t.Spec.InstallationID
131121
}

‎config/crd/bases/github.as-code.io_tokens.yaml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -379,9 +379,6 @@ spec:
379379
type: string
380380
type: object
381381
managedSecret:
382-
description:
383-
'Important: Run "make" to regenerate code after modifying
384-
this file'
385382
properties:
386383
basicAuth:
387384
type: boolean

‎internal/controller/app_controller.go‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,14 @@ func (r *AppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
9494
Reason: failure,
9595
Message: buildErr.Error(),
9696
}
97-
keyValid := metav1.Condition{
98-
Type: githubv1.ConditionTypeKeyValid,
99-
Status: metav1.ConditionFalse,
100-
Reason: failure,
101-
Message: buildErr.Error(),
97+
var keyValid *metav1.Condition
98+
if app.Spec.ValidateKey {
99+
keyValid = &metav1.Condition{
100+
Type: githubv1.ConditionTypeKeyValid,
101+
Status: metav1.ConditionFalse,
102+
Reason: failure,
103+
Message: buildErr.Error(),
104+
}
102105
}
103106
if err := r.writeAppStatus(ctx, app, ready, keyValid); err != nil {
104107
return ctrl.Result{}, err
@@ -112,11 +115,14 @@ func (r *AppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
112115
Reason: githubv1.ReasonReconciled,
113116
Message: "GitHub App client ready",
114117
}
115-
keyValid := metav1.Condition{
116-
Type: githubv1.ConditionTypeKeyValid,
117-
Status: metav1.ConditionTrue,
118-
Reason: githubv1.ReasonReconciled,
119-
Message: "signer key validated",
118+
var keyValid *metav1.Condition
119+
if app.Spec.ValidateKey {
120+
keyValid = &metav1.Condition{
121+
Type: githubv1.ConditionTypeKeyValid,
122+
Status: metav1.ConditionTrue,
123+
Reason: githubv1.ReasonReconciled,
124+
Message: "signer key validated",
125+
}
120126
}
121127
if err := r.writeAppStatus(ctx, app, ready, keyValid); err != nil {
122128
return ctrl.Result{}, err
@@ -125,12 +131,12 @@ func (r *AppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
125131
}
126132

127133
// writeAppStatus applies the Ready condition, applies or clears the KeyValid
128-
// condition based on Spec.ValidateKey, bumps ObservedGeneration, and writes
129-
// status only if anything actually changed.
130-
func (r *AppReconciler) writeAppStatus(ctx context.Context, app *githubv1.App, ready, keyValid metav1.Condition) error {
134+
// condition (nil clears), bumps ObservedGeneration, and writes status only if
135+
// anything actually changed.
136+
func (r *AppReconciler) writeAppStatus(ctx context.Context, app *githubv1.App, ready metav1.Condition, keyValid *metav1.Condition) error {
131137
changed := app.SetStatusCondition(ready)
132-
if app.Spec.ValidateKey {
133-
if app.SetStatusCondition(keyValid) {
138+
if keyValid != nil {
139+
if app.SetStatusCondition(*keyValid) {
134140
changed = true
135141
}
136142
} else if meta.RemoveStatusCondition(&app.Status.Conditions, githubv1.ConditionTypeKeyValid) {

‎internal/controller/reconcile_token.go‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"sigs.k8s.io/controller-runtime/pkg/client"
2424
"sigs.k8s.io/controller-runtime/pkg/log"
2525

26-
githubv1 "github.com/isometry/github-token-manager/api/v1"
2726
"github.com/isometry/github-token-manager/internal/ghapp"
2827
"github.com/isometry/github-token-manager/internal/metrics"
2928
tm "github.com/isometry/github-token-manager/internal/tokenmanager"
@@ -38,20 +37,14 @@ type TokenReconcilerBase struct {
3837
Registry *ghapp.Registry
3938
}
4039

41-
// tokenObject is the generic constraint for a Token-like CR. PT is the
42-
// pointer type (*Token or *ClusterToken); the interface lists the methods
43-
// the reconcile helper needs that aren't already on tm.TokenManager.
44-
type tokenObject[T any] interface {
45-
tm.TokenManager
46-
GetAppRef() *githubv1.AppReference
47-
*T
48-
}
49-
5040
// reconcileTokenLike runs the post-Get reconcile body shared by Token and
5141
// ClusterToken: fetch the typed object, resolve its App reference, surface
5242
// any failure as a status condition, then hand off to tokenmanager to
5343
// reconcile the managed Secret.
54-
func reconcileTokenLike[T any, PT tokenObject[T]](
44+
func reconcileTokenLike[T any, PT interface {
45+
tm.TokenManager
46+
*T
47+
}](
5548
ctx context.Context,
5649
r *TokenReconcilerBase,
5750
req ctrl.Request,
@@ -82,22 +75,14 @@ func reconcileTokenLike[T any, PT tokenObject[T]](
8275
}
8376

8477
options := []tm.Option{
85-
tm.WithReconciler(r.Client),
78+
tm.WithClient(r.Client),
8679
tm.WithGHApp(resolution.Client),
8780
tm.WithLogger(logger),
8881
tm.WithMetrics(r.Metrics),
8982
}
9083

91-
tokenSecret, err := tm.NewTokenSecret(ctx, req.NamespacedName, owner, controllerName, options...)
92-
if err != nil {
93-
logger.Error(err, "failed to create token reconciler")
94-
return ctrl.Result{}, err
95-
}
96-
if tokenSecret == nil {
97-
return ctrl.Result{}, nil
98-
}
99-
100-
result, err := tokenSecret.Reconcile()
84+
tokenSecret := tm.NewTokenSecret(req.NamespacedName, owner, controllerName, options...)
85+
result, err := tokenSecret.Reconcile(ctx)
10186
if err != nil {
10287
logger.Error(err, "failed to reconcile token")
10388
return result, err

‎internal/ghapp/registry.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,6 @@ func (r *Registry) OperatorNamespace() string {
9696
return r.operatorNS
9797
}
9898

99-
// HasStartupConfig reports whether a startup GitHub App config is available.
100-
func (r *Registry) HasStartupConfig() bool {
101-
return r.startupCfg != nil
102-
}
103-
10499
// Startup returns the cached startup-config client, building it on first use.
105100
// Returns [ErrNoStartupConfig] if no startup config was loaded.
106101
func (r *Registry) Startup(ctx context.Context) (ghait.GHAIT, error) {

‎internal/ghapp/registry_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ func countingFactory() (FactoryFunc, *int) {
3535

3636
func TestRegistry_Startup_NoConfig(t *testing.T) {
3737
r := NewRegistry("gtm-system", nil)
38-
if r.HasStartupConfig() {
39-
t.Fatalf("HasStartupConfig() = true with nil cfg")
40-
}
4138
_, err := r.Startup(context.Background())
4239
if !errors.Is(err, ErrNoStartupConfig) {
4340
t.Fatalf("Startup() err = %v, want ErrNoStartupConfig", err)

‎internal/tokenmanager/token_manager.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,12 @@ import (
1010
githubv1 "github.com/isometry/github-token-manager/api/v1"
1111
)
1212

13-
type tokenReconciler interface {
14-
client.Client
15-
}
16-
1713
// TokenManager provides a common interface for both namespaced Tokens and ClusterTokens.
1814
type TokenManager interface {
1915
client.Object
2016

2117
GetType() string
22-
GetName() string
18+
GetAppRef() *githubv1.AppReference
2319
GetSecretBasicAuth() bool
2420
GetInstallationID() int64
2521
GetRefreshInterval() time.Duration

0 commit comments

Comments
 (0)