Skip to content

Commit dc5e0fa

Browse files
ThoSapThoSap
committed
error out if an unsupported privilege is used and conditionally reflect this in the tests
1 parent cc66654 commit dc5e0fa

10 files changed

Lines changed: 386 additions & 27 deletions

File tree

‎operator/src/main/java/it/aboutbits/postgresql/core/Privilege.java‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package it.aboutbits.postgresql.core;
22

33
import com.fasterxml.jackson.annotation.JsonValue;
4+
import lombok.Getter;
5+
import lombok.RequiredArgsConstructor;
6+
import lombok.experimental.Accessors;
47
import org.jooq.impl.DSL;
58
import org.jspecify.annotations.NullMarked;
9+
import org.jspecify.annotations.Nullable;
610

711
import java.util.Locale;
812

@@ -12,19 +16,25 @@
1216
* </a>
1317
*/
1418
@NullMarked
19+
@Getter
20+
@Accessors(fluent = true)
21+
@RequiredArgsConstructor
1522
public enum Privilege {
16-
SELECT,
17-
INSERT,
18-
UPDATE,
19-
DELETE,
20-
TRUNCATE,
21-
REFERENCES,
22-
TRIGGER,
23-
CREATE,
24-
CONNECT,
25-
TEMPORARY,
26-
USAGE;
27-
//MAINTAIN; // PostgreSQL 17+
23+
SELECT(null),
24+
INSERT(null),
25+
UPDATE(null),
26+
DELETE(null),
27+
TRUNCATE(null),
28+
REFERENCES(null),
29+
TRIGGER(null),
30+
CREATE(null),
31+
CONNECT(null),
32+
TEMPORARY(null),
33+
USAGE(null),
34+
MAINTAIN(17);
35+
36+
@Nullable
37+
private final Integer minimumPostgresVersion;
2838

2939
@JsonValue
3040
public String toValue() {

‎operator/src/main/java/it/aboutbits/postgresql/crd/defaultprivilege/DefaultPrivilegeObjectType.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import static it.aboutbits.postgresql.core.Privilege.CREATE;
1515
import static it.aboutbits.postgresql.core.Privilege.DELETE;
1616
import static it.aboutbits.postgresql.core.Privilege.INSERT;
17+
import static it.aboutbits.postgresql.core.Privilege.MAINTAIN;
1718
import static it.aboutbits.postgresql.core.Privilege.REFERENCES;
1819
import static it.aboutbits.postgresql.core.Privilege.SELECT;
1920
import static it.aboutbits.postgresql.core.Privilege.TRIGGER;
@@ -47,8 +48,8 @@ public enum DefaultPrivilegeObjectType {
4748
DELETE,
4849
TRUNCATE,
4950
REFERENCES,
50-
TRIGGER
51-
//MAINTAIN // PostgreSQL 17+
51+
TRIGGER,
52+
MAINTAIN
5253
)
5354
),
5455
SEQUENCE(

‎operator/src/main/java/it/aboutbits/postgresql/crd/defaultprivilege/DefaultPrivilegeReconciler.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import it.aboutbits.postgresql.core.CRPhase;
1111
import it.aboutbits.postgresql.core.CRStatus;
1212
import it.aboutbits.postgresql.core.PostgreSQLContextFactory;
13+
import it.aboutbits.postgresql.core.Privilege;
1314
import lombok.RequiredArgsConstructor;
1415
import lombok.extern.slf4j.Slf4j;
1516
import org.jooq.DSLContext;
@@ -18,6 +19,8 @@
1819
import java.util.HashSet;
1920
import java.util.Set;
2021
import java.util.concurrent.TimeUnit;
22+
import java.util.function.Function;
23+
import java.util.stream.Collectors;
2124

2225
@NullMarked
2326
@Slf4j
@@ -209,7 +212,36 @@ private UpdateControl<DefaultPrivilege> reconcileInTransaction(
209212
) {
210213
var spec = resource.getSpec();
211214

215+
var name = resource.getMetadata().getName();
216+
var namespace = resource.getMetadata().getNamespace();
217+
212218
var expectedPrivileges = Set.copyOf(spec.getPrivileges());
219+
220+
int databaseMajorVersion = tx.connectionResult(connection ->
221+
connection.getMetaData().getDatabaseMajorVersion()
222+
);
223+
224+
var unsupportedPrivileges = expectedPrivileges.stream()
225+
.filter(privilege -> privilege.minimumPostgresVersion() != null
226+
&& databaseMajorVersion < privilege.minimumPostgresVersion()
227+
)
228+
.collect(Collectors.toMap(
229+
Function.identity(),
230+
Privilege::minimumPostgresVersion
231+
));
232+
233+
if (!unsupportedPrivileges.isEmpty()) {
234+
status.setPhase(CRPhase.ERROR)
235+
.setMessage("The following privileges require a newer PostgreSQL version (current: %d): %s [resource=%s/%s]".formatted(
236+
databaseMajorVersion,
237+
unsupportedPrivileges,
238+
getResourceNamespaceOrOwn(resource, namespace),
239+
name
240+
));
241+
242+
return UpdateControl.patchStatus(resource);
243+
}
244+
213245
var currentDefaultPrivileges = defaultPrivilegeService.determineCurrentDefaultPrivileges(tx, spec);
214246

215247
// Calculate Revokes: Current - Expected

‎operator/src/main/java/it/aboutbits/postgresql/crd/grant/GrantObjectType.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import static it.aboutbits.postgresql.core.Privilege.CREATE;
1616
import static it.aboutbits.postgresql.core.Privilege.DELETE;
1717
import static it.aboutbits.postgresql.core.Privilege.INSERT;
18+
import static it.aboutbits.postgresql.core.Privilege.MAINTAIN;
1819
import static it.aboutbits.postgresql.core.Privilege.REFERENCES;
1920
import static it.aboutbits.postgresql.core.Privilege.SELECT;
2021
import static it.aboutbits.postgresql.core.Privilege.TEMPORARY;
@@ -54,8 +55,8 @@ public enum GrantObjectType {
5455
DELETE,
5556
TRUNCATE,
5657
REFERENCES,
57-
TRIGGER
58-
//MAINTAIN // PostgreSQL 17+
58+
TRIGGER,
59+
MAINTAIN
5960
)
6061
),
6162
SEQUENCE(

‎operator/src/main/java/it/aboutbits/postgresql/crd/grant/GrantReconciler.java‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import it.aboutbits.postgresql.core.CRPhase;
1111
import it.aboutbits.postgresql.core.CRStatus;
1212
import it.aboutbits.postgresql.core.PostgreSQLContextFactory;
13+
import it.aboutbits.postgresql.core.Privilege;
1314
import lombok.RequiredArgsConstructor;
1415
import lombok.extern.slf4j.Slf4j;
1516
import org.jooq.DSLContext;
@@ -21,6 +22,8 @@
2122
import java.util.Objects;
2223
import java.util.Set;
2324
import java.util.concurrent.TimeUnit;
25+
import java.util.function.Function;
26+
import java.util.stream.Collectors;
2427

2528
import static org.jooq.impl.DSL.quotedName;
2629

@@ -230,9 +233,34 @@ private UpdateControl<Grant> reconcileInTransaction(
230233
Collections.emptySet()
231234
)
232235
);
236+
var isAllMode = expectedObjects.isEmpty();
237+
233238
var expectedPrivileges = Set.copyOf(spec.getPrivileges());
234239

235-
var isAllMode = expectedObjects.isEmpty();
240+
int databaseMajorVersion = tx.connectionResult(connection ->
241+
connection.getMetaData().getDatabaseMajorVersion()
242+
);
243+
244+
var unsupportedPrivileges = expectedPrivileges.stream()
245+
.filter(privilege -> privilege.minimumPostgresVersion() != null
246+
&& databaseMajorVersion < privilege.minimumPostgresVersion()
247+
)
248+
.collect(Collectors.toMap(
249+
Function.identity(),
250+
Privilege::minimumPostgresVersion
251+
));
252+
253+
if (!unsupportedPrivileges.isEmpty()) {
254+
status.setPhase(CRPhase.ERROR)
255+
.setMessage("The following privileges require a newer PostgreSQL version (current: %d): %s [resource=%s/%s]".formatted(
256+
databaseMajorVersion,
257+
unsupportedPrivileges,
258+
getResourceNamespaceOrOwn(resource, namespace),
259+
name
260+
));
261+
262+
return UpdateControl.patchStatus(resource);
263+
}
236264

237265
var currentObjectPrivileges = grantService.determineCurrentObjectPrivileges(tx, spec);
238266
var ownershipMap = grantService.determineObjectExistenceAndOwnership(tx, spec);

‎operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/ClusterConnectionCreate.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
@Accessors(fluent = true, chain = true)
2323
public class ClusterConnectionCreate extends TestDataCreator<ClusterConnection> {
2424
private final Given given;
25+
2526
private final KubernetesClient kubernetesClient;
2627
private final Given.DBConnectionDetails dbConnectionDetails;
2728

‎operator/src/test/java/it/aboutbits/postgresql/crd/defaultprivilege/DefaultPrivilegeReconcilerErrorTest.java‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
package it.aboutbits.postgresql.crd.defaultprivilege;
2+
3+
import io.fabric8.kubernetes.api.model.HasMetadata;
4+
import io.fabric8.kubernetes.client.KubernetesClient;
5+
import io.quarkus.test.junit.QuarkusTest;
6+
import it.aboutbits.postgresql._support.testdata.persisted.Given;
7+
import it.aboutbits.postgresql.core.CRPhase;
8+
import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnection;
9+
import it.aboutbits.postgresql.crd.database.Database;
10+
import it.aboutbits.postgresql.crd.role.Role;
11+
import it.aboutbits.postgresql.crd.schema.Schema;
12+
import lombok.RequiredArgsConstructor;
13+
import org.junit.jupiter.api.BeforeEach;
14+
import org.junit.jupiter.api.DisplayName;
15+
import org.junit.jupiter.api.Test;
16+
import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
17+
18+
import java.util.concurrent.TimeUnit;
19+
20+
import static it.aboutbits.postgresql.core.Privilege.MAINTAIN;
21+
import static it.aboutbits.postgresql.crd.defaultprivilege.DefaultPrivilegeObjectType.TABLE;
22+
import static org.assertj.core.api.Assertions.assertThat;
23+
24+
@QuarkusTest
25+
@RequiredArgsConstructor
26+
class DefaultPrivilegeReconcilerErrorTest {
27+
private final Given given;
28+
29+
private final KubernetesClient kubernetesClient;
30+
31+
@BeforeEach
32+
void resetEnvironment() {
33+
deleteResources(DefaultPrivilege.class);
34+
deleteResources(Schema.class);
35+
deleteResources(Database.class);
36+
deleteResources(Role.class);
37+
deleteResources(ClusterConnection.class);
38+
39+
// Create the default connection "test-cluster-connection" used by DefaultPrivilegeCreate defaults
40+
given.one().clusterConnection()
41+
.withName("test-cluster-connection")
42+
.returnFirst();
43+
}
44+
45+
private <T extends HasMetadata> void deleteResources(Class<T> resourceClass) {
46+
kubernetesClient.resources(resourceClass)
47+
.withTimeout(5, TimeUnit.SECONDS)
48+
.delete();
49+
}
50+
51+
@Test
52+
@EnabledIfSystemProperty(
53+
named = "quarkus.test.profile",
54+
matches = "test-pg(15|16)",
55+
disabledReason = "PostgreSQL 15 and 16 do not support the MAINTAIN privilege"
56+
)
57+
@DisplayName("Test unsupported MAINTAIN table privilege error")
58+
void testUnsupportedMaintainTablePrivilegeError() {
59+
// given
60+
var clusterConnectionMain = given.one()
61+
.clusterConnection()
62+
.returnFirst();
63+
64+
var database = given.one()
65+
.database()
66+
.withClusterConnectionName(clusterConnectionMain.getMetadata().getName())
67+
.returnFirst();
68+
69+
var clusterConnectionDb = given.one()
70+
.clusterConnection()
71+
.withDatabase(database.getSpec().getName())
72+
.returnFirst();
73+
74+
var schema = given.one()
75+
.schema()
76+
.withClusterConnectionName(clusterConnectionDb.getMetadata().getName())
77+
.returnFirst();
78+
79+
var role = given.one()
80+
.role()
81+
.withClusterConnectionName(clusterConnectionMain.getMetadata().getName())
82+
.returnFirst();
83+
84+
// when
85+
var defaultPrivilege = given.one()
86+
.defaultPrivilege()
87+
.withClusterConnectionName(clusterConnectionDb.getMetadata().getName())
88+
.withDatabase(database.getSpec().getName())
89+
.withSchema(schema.getSpec().getName())
90+
.withRole(role.getSpec().getName())
91+
.withObjectType(TABLE)
92+
.withPrivileges(MAINTAIN)
93+
.returnFirst();
94+
95+
// then
96+
assertThat(defaultPrivilege)
97+
.isNotNull()
98+
.extracting(DefaultPrivilege::getStatus)
99+
.satisfies(status -> {
100+
assertThat(status.getPhase()).isEqualTo(CRPhase.ERROR);
101+
assertThat(status.getMessage()).startsWith("The following privileges require a newer PostgreSQL version (current: 16): {MAINTAIN=17}");
102+
});
103+
}
104+
}

‎operator/src/test/java/it/aboutbits/postgresql/crd/defaultprivilege/DefaultPrivilegeReconcilerTest.java‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,16 @@
2121
import org.junit.jupiter.api.Nested;
2222
import org.junit.jupiter.api.Test;
2323
import org.junit.jupiter.params.ParameterizedTest;
24+
import org.junit.jupiter.params.provider.MethodSource;
2425

2526
import java.time.OffsetDateTime;
2627
import java.time.ZoneOffset;
2728
import java.util.List;
2829
import java.util.Set;
2930
import java.util.concurrent.TimeUnit;
3031
import java.util.function.Predicate;
32+
import java.util.regex.Pattern;
33+
import java.util.stream.Stream;
3134

3235
import static it.aboutbits.postgresql.core.Privilege.CREATE;
3336
import static it.aboutbits.postgresql.core.Privilege.SELECT;
@@ -43,8 +46,10 @@
4346
@RequiredArgsConstructor
4447
class DefaultPrivilegeReconcilerTest {
4548
private final Given given;
49+
4650
private final DefaultPrivilegeService defaultPrivilegeService;
4751
private final PostgreSQLContextFactory postgreSQLContextFactory;
52+
4853
private final KubernetesClient kubernetesClient;
4954

5055
@BeforeEach
@@ -438,15 +443,19 @@ void defaultPrivilegeOnSchema() {
438443

439444
@Nested
440445
class TableTests {
441-
@Test
446+
@ParameterizedTest
447+
@MethodSource("provideAllSupportedPrivileges")
442448
@DisplayName("Should grant and revoke default privileges on table")
443-
void defaultPrivilegeOnTable() {
449+
void defaultPrivilegeOnTable(
450+
List<Privilege> allSupportedPrivileges
451+
) {
444452
// given
445453
var now = OffsetDateTime.now(ZoneOffset.UTC);
446454

447455
var clusterConnectionMain = given.one()
448456
.clusterConnection()
449457
.returnFirst();
458+
450459
var database = given.one()
451460
.database()
452461
.withClusterConnectionName(clusterConnectionMain.getMetadata().getName())
@@ -498,7 +507,7 @@ void defaultPrivilegeOnTable() {
498507

499508
// given: grant all default privileges change
500509
var spec = defaultPrivilege.getSpec();
501-
var expectedPrivileges = TABLE.privileges();
510+
var expectedPrivileges = allSupportedPrivileges;
502511
var initialGeneration = defaultPrivilege.getStatus().getObservedGeneration();
503512

504513
spec.setPrivileges(expectedPrivileges);
@@ -544,6 +553,19 @@ void defaultPrivilegeOnTable() {
544553
defaultPrivilege
545554
);
546555
}
556+
557+
static Stream<List<Privilege>> provideAllSupportedPrivileges() {
558+
var profile = System.getProperty("quarkus.test.profile", "");
559+
var matcher = Pattern.compile("test-pg(\\d+)").matcher(profile);
560+
int version = matcher.find() ? Integer.parseInt(matcher.group(1)) : 0;
561+
562+
return Stream.of(TABLE.privilegesSet().stream()
563+
.filter(privilege -> privilege.minimumPostgresVersion() == null
564+
|| privilege.minimumPostgresVersion() <= version
565+
)
566+
.toList()
567+
);
568+
}
547569
}
548570

549571
@Nested
@@ -557,6 +579,7 @@ void defaultPrivilegeOnSequence() {
557579
var clusterConnectionMain = given.one()
558580
.clusterConnection()
559581
.returnFirst();
582+
560583
var database = given.one()
561584
.database()
562585
.withClusterConnectionName(clusterConnectionMain.getMetadata().getName())

0 commit comments

Comments
 (0)