Skip to content

Commit a131ea9

Browse files
ThoSapThoSap
committed
implement the review feedback and implement the role cleanup when a Role CR instance is deleted (also in the tests)
1 parent 8616b99 commit a131ea9

13 files changed

Lines changed: 439 additions & 227 deletions

File tree

src/main/java/it/aboutbits/postgresql/core/KubernetesUtil.java renamed to src/main/java/it/aboutbits/postgresql/core/KubernetesService.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,20 @@
22

33
import io.fabric8.kubernetes.client.KubernetesClient;
44
import it.aboutbits.postgresql.crd.connection.ClusterConnection;
5+
import jakarta.inject.Singleton;
56
import org.jspecify.annotations.NullMarked;
67

78
import java.nio.charset.Charset;
89
import java.util.Base64;
910

1011
@NullMarked
11-
public final class KubernetesUtil {
12+
@Singleton
13+
public final class KubernetesService {
1214
public static final String SECRET_TYPE_BASIC_AUTH = "kubernetes.io/basic-auth";
1315
public static final String SECRET_DATA_BASIC_AUTH_USERNAME_KEY = "username";
1416
public static final String SECRET_DATA_BASIC_AUTH_PASSWORD_KEY = "password";
1517

16-
public static Credentials getSecretRefCredentials(
18+
public Credentials getSecretRefCredentials(
1719
KubernetesClient kubernetesClient,
1820
ClusterConnection clusterConnection
1921
) {
@@ -24,7 +26,7 @@ public static Credentials getSecretRefCredentials(
2426
);
2527
}
2628

27-
public static Credentials getSecretRefCredentials(
29+
public Credentials getSecretRefCredentials(
2830
KubernetesClient kubernetesClient,
2931
SecretRef secretRef,
3032
String defaultNamespace
@@ -89,7 +91,4 @@ public static Credentials getSecretRefCredentials(
8991
password
9092
);
9193
}
92-
93-
private KubernetesUtil() {
94-
}
9594
}

src/main/java/it/aboutbits/postgresql/core/PostgreSQLAuthenticationUtil.java renamed to src/main/java/it/aboutbits/postgresql/core/PostgreSQLAuthenticationService.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.ongres.scram.common.StringPreparation;
44
import it.aboutbits.postgresql.crd.role.RoleSpec;
5+
import jakarta.inject.Singleton;
56
import lombok.extern.slf4j.Slf4j;
67
import org.jooq.DSLContext;
78
import org.jspecify.annotations.NullMarked;
@@ -22,13 +23,14 @@
2223

2324
@NullMarked
2425
@Slf4j
25-
public final class PostgreSQLAuthenticationUtil {
26+
@Singleton
27+
public final class PostgreSQLAuthenticationService {
2628
private static final String MD5 = "MD5";
2729
private static final String SHA_256 = "SHA-256";
2830
private static final String HMAC_SHA_256 = "HmacSHA256";
2931
private static final String PBKDF2_WITH_HMAC_SHA256 = "PBKDF2WithHmacSHA256";
3032

31-
public static boolean passwordMatches(
33+
public boolean passwordMatches(
3234
DSLContext dsl,
3335
RoleSpec spec,
3436
String expectedPassword
@@ -214,7 +216,4 @@ private static byte[] sha256(byte[] data) {
214216
throw new IllegalStateException("%s not available".formatted(SHA_256), e);
215217
}
216218
}
217-
218-
private PostgreSQLAuthenticationUtil() {
219-
}
220219
}

‎src/main/java/it/aboutbits/postgresql/core/PostgreSQLContextFactory.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import io.fabric8.kubernetes.client.KubernetesClient;
44
import it.aboutbits.postgresql.crd.connection.ClusterConnection;
55
import jakarta.enterprise.context.ApplicationScoped;
6+
import lombok.RequiredArgsConstructor;
67
import org.jooq.CloseableDSLContext;
78
import org.jooq.impl.DSL;
89
import org.jspecify.annotations.NullMarked;
@@ -11,18 +12,16 @@
1112

1213
@NullMarked
1314
@ApplicationScoped
15+
@RequiredArgsConstructor
1416
public class PostgreSQLContextFactory {
1517
private static final String POSTGRESQL_AUTHENTICATION_USER_KEY = "user";
1618
private static final String POSTGRESQL_AUTHENTICATION_PASSWORD_KEY = "password";
1719

20+
private final KubernetesService kubernetesService;
1821
private final KubernetesClient kubernetesClient;
1922

20-
public PostgreSQLContextFactory(KubernetesClient kubernetesClient) {
21-
this.kubernetesClient = kubernetesClient;
22-
}
23-
2423
public CloseableDSLContext getDSLContext(ClusterConnection clusterConnection) {
25-
var credentials = KubernetesUtil.getSecretRefCredentials(
24+
var credentials = kubernetesService.getSecretRefCredentials(
2625
kubernetesClient,
2726
clusterConnection
2827
);

‎src/main/java/it/aboutbits/postgresql/core/SQLUtil.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package it.aboutbits.postgresql.core;
2+
3+
import org.jooq.QueryPart;
4+
import org.jspecify.annotations.NullMarked;
5+
6+
import java.util.List;
7+
8+
import static org.jooq.impl.DSL.sql;
9+
10+
@NullMarked
11+
public final class SQLUtil {
12+
public static QueryPart concatenateQueryPartsWithSpaces(List<? extends QueryPart> parts) {
13+
return concatenateQueryParts(parts, " ");
14+
}
15+
16+
public static QueryPart concatenateQueryPartsWithComma(List<? extends QueryPart> parts) {
17+
return concatenateQueryParts(parts, ", ");
18+
}
19+
20+
/**
21+
* Concatenate QueryParts with the requested separator
22+
*/
23+
private static QueryPart concatenateQueryParts(
24+
List<? extends QueryPart> items,
25+
String separator
26+
) {
27+
int size = items.size();
28+
29+
if (items.isEmpty()) {
30+
return sql("");
31+
} else if (size == 1) {
32+
return items.getFirst();
33+
}
34+
35+
var template = new StringBuilder();
36+
37+
// Add the first item without a separator
38+
template.append('{').append(0).append('}');
39+
40+
// Add the rest of the items with the leading separator
41+
for (int i = 1; i < size; i++) {
42+
template.append(separator).append('{').append(i).append('}');
43+
}
44+
45+
return sql(
46+
template.toString(),
47+
items.toArray(QueryPart[]::new)
48+
);
49+
}
50+
51+
private SQLUtil() {
52+
}
53+
}

src/main/java/it/aboutbits/postgresql/crd/role/RoleFlags.java renamed to src/main/java/it/aboutbits/postgresql/crd/role/RoleFlag.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
@Getter
1010
@Accessors(fluent = true)
1111
@RequiredArgsConstructor
12-
public enum RoleFlags {
12+
public enum RoleFlag {
1313
SUPERUSER("SUPERUSER"),
1414
NO_SUPERUSER("NOSUPERUSER"),
1515

‎src/main/java/it/aboutbits/postgresql/crd/role/RoleReconciler.java‎

Lines changed: 83 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import io.fabric8.kubernetes.api.model.Secret;
44
import io.fabric8.kubernetes.client.KubernetesClient;
55
import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration;
6+
import io.javaoperatorsdk.operator.api.reconciler.Cleaner;
67
import io.javaoperatorsdk.operator.api.reconciler.Context;
8+
import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
79
import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
810
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
911
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
@@ -14,8 +16,8 @@
1416
import it.aboutbits.postgresql.core.BaseReconciler;
1517
import it.aboutbits.postgresql.core.CRPhase;
1618
import it.aboutbits.postgresql.core.CRStatus;
17-
import it.aboutbits.postgresql.core.KubernetesUtil;
18-
import it.aboutbits.postgresql.core.PostgreSQLAuthenticationUtil;
19+
import it.aboutbits.postgresql.core.KubernetesService;
20+
import it.aboutbits.postgresql.core.PostgreSQLAuthenticationService;
1921
import it.aboutbits.postgresql.core.PostgreSQLContextFactory;
2022
import lombok.RequiredArgsConstructor;
2123
import lombok.extern.slf4j.Slf4j;
@@ -32,7 +34,11 @@
3234
@RequiredArgsConstructor
3335
public class RoleReconciler
3436
extends BaseReconciler<Role, CRStatus>
35-
implements Reconciler<Role> {
37+
implements Reconciler<Role>, Cleaner<Role> {
38+
private final RoleService roleService;
39+
private final KubernetesService kubernetesService;
40+
private final PostgreSQLAuthenticationService postgreSQLAuthenticationService;
41+
3642
private final KubernetesClient kubernetesClient;
3743
private final PostgreSQLContextFactory contextFactory;
3844

@@ -84,7 +90,7 @@ public UpdateControl<Role> reconcile(
8490

8591
String password;
8692
if (passwordSecretRef != null) {
87-
password = KubernetesUtil.getSecretRefCredentials(
93+
password = kubernetesService.getSecretRefCredentials(
8894
kubernetesClient,
8995
passwordSecretRef,
9096
namespace
@@ -116,6 +122,70 @@ public UpdateControl<Role> reconcile(
116122
return updateControl;
117123
}
118124

125+
@Override
126+
public DeleteControl cleanup(
127+
Role resource,
128+
Context<Role> context
129+
) {
130+
var spec = resource.getSpec();
131+
var status = initializeStatus(resource);
132+
133+
var name = resource.getMetadata().getName();
134+
var namespace = resource.getMetadata().getNamespace();
135+
136+
log.info(
137+
"Deleting Role [resource={}/{}, spec.name={}, status.phase={}]",
138+
namespace,
139+
name,
140+
spec.getName(),
141+
status.getPhase()
142+
);
143+
144+
if (status.getPhase() != CRPhase.DELETING) {
145+
status.setPhase(CRPhase.DELETING)
146+
.setMessage("Role deletion in progress");
147+
}
148+
149+
var clusterRef = spec.getClusterRef();
150+
151+
var clusterConnectionOptional = getReferencedClusterConnection(
152+
kubernetesClient,
153+
resource,
154+
clusterRef
155+
);
156+
157+
if (clusterConnectionOptional.isEmpty()) {
158+
status.setMessage("The specified ClusterConnection no longer exists or is not ready yet [clusterRef=%s/%s]".formatted(
159+
getResourceNamespaceOrOwn(resource, clusterRef.getNamespace()),
160+
clusterRef.getName()
161+
));
162+
163+
return DeleteControl.noFinalizerRemoval()
164+
.rescheduleAfter(60, TimeUnit.SECONDS);
165+
}
166+
167+
var clusterConnection = clusterConnectionOptional.get();
168+
169+
try (var dsl = contextFactory.getDSLContext(clusterConnection)) {
170+
roleService.dropRole(dsl, spec);
171+
172+
return DeleteControl.defaultDelete();
173+
} catch (Exception e) {
174+
log.error(
175+
"Failed to delete Role [resource={}/{}, spec.name={}, status.phase={}]",
176+
namespace,
177+
name,
178+
spec.getName(),
179+
status.getPhase()
180+
);
181+
182+
status.setMessage("Deletion failed: " + e.getMessage());
183+
184+
return DeleteControl.noFinalizerRemoval()
185+
.rescheduleAfter(60, TimeUnit.SECONDS);
186+
}
187+
}
188+
119189
/**
120190
* Watches for {@code Secret} changes to trigger reconciliation for dependent {@code Role} resources.
121191
*/
@@ -160,14 +230,14 @@ private UpdateControl<Role> reconcileInTransaction(
160230
var expectedFlags = spec.getFlags();
161231

162232
// Create and return the role if it doesn't exist yet
163-
if (!RoleUtil.roleExists(tx, spec)) {
233+
if (!roleService.roleExists(tx, spec)) {
164234
log.info(
165235
"Creating Role [resource={}/{}]",
166236
namespace,
167237
name
168238
);
169239

170-
RoleUtil.createRole(
240+
roleService.createRole(
171241
tx,
172242
spec,
173243
password
@@ -181,16 +251,16 @@ private UpdateControl<Role> reconcileInTransaction(
181251

182252
// When there is NOLOGIN, we set no password
183253
var passwordMatches = true;
184-
var roleLoginMatches = RoleUtil.roleLoginMatches(tx, spec);
185-
var currentFlags = RoleUtil.fetchCurrentFlags(tx, spec);
254+
var roleLoginMatches = roleService.roleLoginMatches(tx, spec);
255+
var currentFlags = roleService.fetchCurrentFlags(tx, spec);
186256
var flagsMatch = expectedFlags.equals(currentFlags);
187-
var commentMatches = RoleUtil.roleCommentMatches(tx, spec);
257+
var commentMatches = roleService.roleCommentMatches(tx, spec);
188258

189259
var passwordSecretRef = spec.getPasswordSecretRef();
190260
var loginExpected = passwordSecretRef != null;
191261

192262
if (loginExpected && password != null) {
193-
passwordMatches = PostgreSQLAuthenticationUtil.passwordMatches(
263+
passwordMatches = postgreSQLAuthenticationService.passwordMatches(
194264
tx,
195265
spec,
196266
password
@@ -216,7 +286,7 @@ private UpdateControl<Role> reconcileInTransaction(
216286
);
217287

218288
if (!roleLoginMatches || !passwordMatches || !flagsMatch) {
219-
RoleUtil.alterRole(
289+
roleService.alterRole(
220290
tx,
221291
spec,
222292
changePassword,
@@ -231,7 +301,7 @@ private UpdateControl<Role> reconcileInTransaction(
231301
name
232302
);
233303

234-
RoleUtil.reconcileRoleMembership(
304+
roleService.reconcileRoleMembership(
235305
tx,
236306
spec,
237307
expectedFlags,
@@ -240,7 +310,7 @@ private UpdateControl<Role> reconcileInTransaction(
240310
}
241311

242312
if (!commentMatches) {
243-
RoleUtil.updateComment(
313+
roleService.updateComment(
244314
tx,
245315
spec
246316
);

0 commit comments

Comments
 (0)