Skip to content

Commit 0ebdef0

Browse files
cbandyTony Landreth
cbandy
and
Tony Landreth
committed
Consolidate registration logic and add more tests
A single condition, Registered=False, appears consistently on different API objects. Messages on conditions and events are defined together in one file. The "REGISTRATION_REQUIRED" environment variable no longer has any effect. Co-authored-by: Chris Bandy <chris.bandy@crunchydata.com> Co-authored-by: Tony Landreth <tony.landreth@crunchydata.com> Issue: PGO-700 Issue: PGO-825 Issue: PGO-979 Issue: PGO-986 Issue: PGO-1050 Issue: PGO-1105
1 parent 7ecee7e commit 0ebdef0

File tree

22 files changed

+1081
-505
lines changed

22 files changed

+1081
-505
lines changed

‎cmd/postgres-operator/main.go

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ limitations under the License.
1616
*/
1717

1818
import (
19+
"context"
1920
"net/http"
2021
"os"
2122
"strings"
@@ -35,6 +36,7 @@ import (
3536
"github.com/crunchydata/postgres-operator/internal/controller/standalone_pgadmin"
3637
"github.com/crunchydata/postgres-operator/internal/logging"
3738
"github.com/crunchydata/postgres-operator/internal/naming"
39+
"github.com/crunchydata/postgres-operator/internal/registration"
3840
"github.com/crunchydata/postgres-operator/internal/upgradecheck"
3941
"github.com/crunchydata/postgres-operator/internal/util"
4042
)
@@ -70,6 +72,7 @@ func main() {
7072

7173
// create a context that will be used to stop all controllers on a SIGTERM or SIGINT
7274
ctx := cruntime.SetupSignalHandler()
75+
ctx, shutdown := context.WithCancel(ctx)
7376
log := logging.FromContext(ctx)
7477
log.V(1).Info("debug flag set to true")
7578

@@ -96,8 +99,13 @@ func main() {
9699
log.Info("detected OpenShift environment")
97100
}
98101

102+
registrar, err := registration.NewRunner(os.Getenv("RSA_KEY"), os.Getenv("TOKEN_PATH"), shutdown)
103+
assertNoError(err)
104+
assertNoError(mgr.Add(registrar))
105+
_ = registrar.CheckToken()
106+
99107
// add all PostgreSQL Operator controllers to the runtime manager
100-
addControllersToManager(mgr, openshift, log)
108+
addControllersToManager(mgr, openshift, log, registrar)
101109

102110
if util.DefaultMutableFeatureGate.Enabled(util.BridgeIdentifiers) {
103111
constructor := func() *bridge.Client {
@@ -128,23 +136,14 @@ func main() {
128136

129137
// addControllersToManager adds all PostgreSQL Operator controllers to the provided controller
130138
// runtime manager.
131-
func addControllersToManager(mgr manager.Manager, openshift bool, log logr.Logger) {
132-
semanticVersionString := util.SemanticMajorMinorPatch(versionString)
133-
if semanticVersionString == "" {
134-
os.Setenv("REGISTRATION_REQUIRED", "false")
135-
}
136-
139+
func addControllersToManager(mgr manager.Manager, openshift bool, log logr.Logger, reg registration.Registration) {
137140
pgReconciler := &postgrescluster.Reconciler{
138-
Client: mgr.GetClient(),
139-
IsOpenShift: openshift,
140-
Owner: postgrescluster.ControllerName,
141-
PGOVersion: semanticVersionString,
142-
Recorder: mgr.GetEventRecorderFor(postgrescluster.ControllerName),
143-
// TODO(tlandreth) Replace the contents of cpk_rsa_key.pub with a key from a
144-
// Crunchy authorization server.
145-
Registration: util.GetRegistration(os.Getenv("RSA_KEY"), os.Getenv("TOKEN_PATH"), log),
146-
RegistrationURL: os.Getenv("REGISTRATION_URL"),
147-
Tracer: otel.Tracer(postgrescluster.ControllerName),
141+
Client: mgr.GetClient(),
142+
IsOpenShift: openshift,
143+
Owner: postgrescluster.ControllerName,
144+
Recorder: mgr.GetEventRecorderFor(postgrescluster.ControllerName),
145+
Registration: reg,
146+
Tracer: otel.Tracer(postgrescluster.ControllerName),
148147
}
149148

150149
if err := pgReconciler.SetupWithManager(mgr); err != nil {
@@ -153,9 +152,10 @@ func addControllersToManager(mgr manager.Manager, openshift bool, log logr.Logge
153152
}
154153

155154
upgradeReconciler := &pgupgrade.PGUpgradeReconciler{
156-
Client: mgr.GetClient(),
157-
Owner: "pgupgrade-controller",
158-
Scheme: mgr.GetScheme(),
155+
Client: mgr.GetClient(),
156+
Owner: "pgupgrade-controller",
157+
Recorder: mgr.GetEventRecorderFor("pgupgrade-controller"),
158+
Registration: reg,
159159
}
160160

161161
if err := upgradeReconciler.SetupWithManager(mgr); err != nil {

‎config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15457,8 +15457,6 @@ spec:
1545715457
type: object
1545815458
type: object
1545915459
registrationRequired:
15460-
description: Version information for installations with a registration
15461-
requirement.
1546215460
properties:
1546315461
pgoVersion:
1546415462
type: string
@@ -15471,8 +15469,6 @@ spec:
1547115469
description: The instance set associated with the startupInstance
1547215470
type: string
1547315471
tokenRequired:
15474-
description: Signals the need for a token to be applied when registration
15475-
is required.
1547615472
type: string
1547715473
userInterface:
1547815474
description: Current state of the PostgreSQL user interface.

‎go.mod

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ go 1.19
55
require (
66
github.com/evanphx/json-patch/v5 v5.6.0
77
github.com/go-logr/logr v1.3.0
8-
github.com/golang-jwt/jwt/v5 v5.0.0
8+
github.com/golang-jwt/jwt/v5 v5.2.1
99
github.com/google/go-cmp v0.5.9
1010
github.com/google/uuid v1.3.1
1111
github.com/onsi/ginkgo/v2 v2.0.0
@@ -21,7 +21,6 @@ require (
2121
go.opentelemetry.io/otel/sdk v1.2.0
2222
go.opentelemetry.io/otel/trace v1.19.0
2323
golang.org/x/crypto v0.19.0
24-
golang.org/x/mod v0.8.0
2524
gotest.tools/v3 v3.1.0
2625
k8s.io/api v0.24.2
2726
k8s.io/apimachinery v0.24.2

‎go.sum

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,8 @@ github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zV
192192
github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o=
193193
github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
194194
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
195-
github.com/golang-jwt/jwt/v5 v5.0.0 h1:1n1XNM9hk7O9mnQoNBGolZvzebBQ7p93ULHRc28XJUE=
196-
github.com/golang-jwt/jwt/v5 v5.0.0/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
195+
github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk=
196+
github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
197197
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
198198
github.com/golang/glog v1.0.0/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4=
199199
github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
@@ -602,8 +602,6 @@ golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
602602
golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
603603
golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
604604
golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
605-
golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8=
606-
golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
607605
golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
608606
golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
609607
golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=

‎internal/config/config.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,18 +50,6 @@ func FetchKeyCommand(spec *v1beta1.PostgresClusterSpec) string {
5050
return ""
5151
}
5252

53-
func RegistrationRequired() bool {
54-
return os.Getenv("REGISTRATION_REQUIRED") == "true"
55-
}
56-
57-
// Get the version of CPK that applied the first RegistrationRequired status to this cluster.
58-
func RegistrationRequiredBy(cluster *v1beta1.PostgresCluster) string {
59-
if cluster.Status.RegistrationRequired == nil {
60-
return ""
61-
}
62-
return cluster.Status.RegistrationRequired.PGOVersion
63-
}
64-
6553
// Red Hat Marketplace requires operators to use environment variables be used
6654
// for any image other than the operator itself. Those variables must start with
6755
// "RELATED_IMAGE_" so that OSBS can transform their tag values into digests

‎internal/controller/pgupgrade/pgupgrade_controller.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
"k8s.io/apimachinery/pkg/api/equality"
2424
"k8s.io/apimachinery/pkg/api/meta"
2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26-
"k8s.io/apimachinery/pkg/runtime"
26+
"k8s.io/client-go/tools/record"
2727
"k8s.io/client-go/util/workqueue"
2828
ctrl "sigs.k8s.io/controller-runtime"
2929
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -32,6 +32,7 @@ import (
3232
"sigs.k8s.io/controller-runtime/pkg/source"
3333

3434
"github.com/crunchydata/postgres-operator/internal/config"
35+
"github.com/crunchydata/postgres-operator/internal/registration"
3536
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
3637
)
3738

@@ -41,14 +42,11 @@ const (
4142

4243
// PGUpgradeReconciler reconciles a PGUpgrade object
4344
type PGUpgradeReconciler struct {
44-
client.Client
45+
Client client.Client
4546
Owner client.FieldOwner
46-
Scheme *runtime.Scheme
4747

48-
// For this iteration, we will only be setting conditions rather than
49-
// setting conditions and emitting events. That may change in the future,
50-
// so we're leaving this EventRecorder here for now.
51-
// record.EventRecorder
48+
Recorder record.EventRecorder
49+
Registration registration.Registration
5250
}
5351

5452
//+kubebuilder:rbac:groups="batch",resources="jobs",verbs={list,watch}
@@ -80,7 +78,7 @@ func (r *PGUpgradeReconciler) findUpgradesForPostgresCluster(
8078
// namespace, we can configure the [ctrl.Manager] field indexer and pass a
8179
// [fields.Selector] here.
8280
// - https://book.kubebuilder.io/reference/watching-resources/externally-managed.html
83-
if r.List(ctx, &upgrades, &client.ListOptions{
81+
if r.Client.List(ctx, &upgrades, &client.ListOptions{
8482
Namespace: cluster.Namespace,
8583
}) == nil {
8684
for i := range upgrades.Items {
@@ -140,14 +138,14 @@ func (r *PGUpgradeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
140138
// copy before returning from its cache.
141139
// - https://github.com/kubernetes-sigs/controller-runtime/issues/1235
142140
upgrade := &v1beta1.PGUpgrade{}
143-
err = r.Get(ctx, req.NamespacedName, upgrade)
141+
err = r.Client.Get(ctx, req.NamespacedName, upgrade)
144142

145143
if err == nil {
146144
// Write any changes to the upgrade status on the way out.
147145
before := upgrade.DeepCopy()
148146
defer func() {
149147
if !equality.Semantic.DeepEqual(before.Status, upgrade.Status) {
150-
status := r.Status().Patch(ctx, upgrade, client.MergeFrom(before), r.Owner)
148+
status := r.Client.Status().Patch(ctx, upgrade, client.MergeFrom(before), r.Owner)
151149

152150
if err == nil && status != nil {
153151
err = status
@@ -178,6 +176,10 @@ func (r *PGUpgradeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
178176
return
179177
}
180178

179+
if !r.UpgradeAuthorized(upgrade) {
180+
return ctrl.Result{}, nil
181+
}
182+
181183
// Set progressing condition to true if it doesn't exist already
182184
setStatusToProgressingIfReasonWas("", upgrade)
183185

@@ -452,7 +454,7 @@ func (r *PGUpgradeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
452454
// Set the pgBackRest status for bootstrapping
453455
patch.Status.PGBackRest.Repos = []v1beta1.RepoStatus{}
454456

455-
err = r.Status().Patch(ctx, patch, client.MergeFrom(world.Cluster), r.Owner)
457+
err = r.Client.Status().Patch(ctx, patch, client.MergeFrom(world.Cluster), r.Owner)
456458
}
457459

458460
return ctrl.Result{}, err

‎internal/controller/pgupgrade/registration.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2021 - 2024 Crunchy Data Solutions, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package pgupgrade
16+
17+
import (
18+
"k8s.io/apimachinery/pkg/api/meta"
19+
20+
"github.com/crunchydata/postgres-operator/internal/registration"
21+
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
22+
)
23+
24+
func (r *PGUpgradeReconciler) UpgradeAuthorized(upgrade *v1beta1.PGUpgrade) bool {
25+
// Allow an upgrade in progress to complete, when the registration requirement is introduced.
26+
// But don't allow new upgrades to be started until a valid token is applied.
27+
progressing := meta.FindStatusCondition(upgrade.Status.Conditions, ConditionPGUpgradeProgressing) != nil
28+
required := r.Registration.Required(r.Recorder, upgrade, &upgrade.Status.Conditions)
29+
30+
// If a valid token has not been applied, warn the user.
31+
if required && !progressing {
32+
registration.SetRequiredWarning(r.Recorder, upgrade, &upgrade.Status.Conditions)
33+
return false
34+
}
35+
36+
return true
37+
}

‎internal/controller/pgupgrade/registration_test.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Copyright 2021 - 2024 Crunchy Data Solutions, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package pgupgrade
16+
17+
import (
18+
"testing"
19+
20+
"gotest.tools/v3/assert"
21+
"k8s.io/apimachinery/pkg/api/meta"
22+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
23+
"k8s.io/client-go/tools/record"
24+
"sigs.k8s.io/controller-runtime/pkg/client"
25+
26+
"github.com/crunchydata/postgres-operator/internal/controller/runtime"
27+
"github.com/crunchydata/postgres-operator/internal/registration"
28+
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
29+
"github.com/crunchydata/postgres-operator/internal/testing/events"
30+
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
31+
)
32+
33+
func TestUpgradeAuthorized(t *testing.T) {
34+
t.Run("UpgradeAlreadyInProgress", func(t *testing.T) {
35+
reconciler := new(PGUpgradeReconciler)
36+
upgrade := new(v1beta1.PGUpgrade)
37+
38+
for _, required := range []bool{false, true} {
39+
reconciler.Registration = registration.RegistrationFunc(
40+
func(record.EventRecorder, client.Object, *[]metav1.Condition) bool {
41+
return required
42+
})
43+
44+
meta.SetStatusCondition(&upgrade.Status.Conditions, metav1.Condition{
45+
Type: ConditionPGUpgradeProgressing,
46+
Status: metav1.ConditionTrue,
47+
})
48+
49+
result := reconciler.UpgradeAuthorized(upgrade)
50+
assert.Assert(t, result, "expected signal to proceed")
51+
52+
progressing := meta.FindStatusCondition(upgrade.Status.Conditions, ConditionPGUpgradeProgressing)
53+
assert.Equal(t, progressing.Status, metav1.ConditionTrue)
54+
}
55+
})
56+
57+
t.Run("RegistrationRequired", func(t *testing.T) {
58+
scheme, err := runtime.CreatePostgresOperatorScheme()
59+
assert.NilError(t, err)
60+
61+
recorder := events.NewRecorder(t, scheme)
62+
upgrade := new(v1beta1.PGUpgrade)
63+
upgrade.Name = "some-upgrade"
64+
65+
reconciler := PGUpgradeReconciler{
66+
Recorder: recorder,
67+
Registration: registration.RegistrationFunc(
68+
func(record.EventRecorder, client.Object, *[]metav1.Condition) bool {
69+
return true
70+
}),
71+
}
72+
73+
meta.RemoveStatusCondition(&upgrade.Status.Conditions, ConditionPGUpgradeProgressing)
74+
75+
result := reconciler.UpgradeAuthorized(upgrade)
76+
assert.Assert(t, !result, "expected signal to not proceed")
77+
78+
condition := meta.FindStatusCondition(upgrade.Status.Conditions, v1beta1.Registered)
79+
if assert.Check(t, condition != nil) {
80+
assert.Equal(t, condition.Status, metav1.ConditionFalse)
81+
}
82+
83+
if assert.Check(t, len(recorder.Events) > 0) {
84+
assert.Equal(t, recorder.Events[0].Type, "Warning")
85+
assert.Equal(t, recorder.Events[0].Regarding.Kind, "PGUpgrade")
86+
assert.Equal(t, recorder.Events[0].Regarding.Name, "some-upgrade")
87+
assert.Assert(t, cmp.Contains(recorder.Events[0].Note, "requires"))
88+
}
89+
})
90+
91+
t.Run("RegistrationCompleted", func(t *testing.T) {
92+
reconciler := new(PGUpgradeReconciler)
93+
upgrade := new(v1beta1.PGUpgrade)
94+
95+
called := false
96+
reconciler.Registration = registration.RegistrationFunc(
97+
func(record.EventRecorder, client.Object, *[]metav1.Condition) bool {
98+
called = true
99+
return false
100+
})
101+
102+
meta.RemoveStatusCondition(&upgrade.Status.Conditions, ConditionPGUpgradeProgressing)
103+
104+
result := reconciler.UpgradeAuthorized(upgrade)
105+
assert.Assert(t, result, "expected signal to proceed")
106+
assert.Assert(t, called, "expected registration package to clear conditions")
107+
})
108+
}

0 commit comments

Comments
 (0)