Skip to content

Commit 0b1abf1

Browse files
vshnbotvshnbot
authored
Merge pull request #525 from vshn/fix/user-management-sequence
Sequence user management for postgresql and mariadb
2 parents c3ae7fd + 6e141ea commit 0b1abf1

File tree

5 files changed

+127
-22
lines changed

5 files changed

+127
-22
lines changed

‎pkg/comp-functions/functions/vshnmariadb/user_management.go‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,41 @@ func UserManagement(ctx context.Context, comp *vshnv1.VSHNMariaDB, svc *runtime.
3131

3232
for _, access := range comp.Spec.Parameters.Service.Access {
3333

34+
// We don't have a dependency for the user
35+
// Checking for existence of the helm release does not mean that mariadb is ready to accept connections
3436
userPasswordRef := addUser(comp, svc, *access.User)
3537

3638
dbname := *access.User
3739
if access.Database != nil {
3840
dbname = *access.Database
3941
}
4042

41-
addDatabase(comp, svc, dbname)
43+
userName := fmt.Sprintf("%s-%s-role", comp.GetName(), *access.User)
44+
dbName := fmt.Sprintf("%s-%s-database", comp.GetName(), dbname)
45+
grantName := fmt.Sprintf("%s-%s-%s-grants", comp.GetName(), *access.User, dbname)
46+
47+
// Only add database if user is ready
48+
// We also check for self-existence to prevent deletion when the depending resource turns unready
49+
userReady, userErr := svc.IsResourceReady(userName)
50+
if (userErr == nil && userReady) || svc.ResourceExistsInObserved(dbName) {
51+
addDatabase(comp, svc, dbname)
52+
} else if userErr != nil {
53+
svc.Log.Info("User not yet ready, skipping database creation", "user", userName, "error", userErr.Error())
54+
}
4255

43-
addGrants(comp, svc, *access.User, dbname, access.Privileges)
56+
// Only add grants if both user and database are ready
57+
// We also check for self-existence to prevent deletion when the depending resource turns unready
58+
dbReady, dbErr := svc.IsResourceReady(dbName)
59+
if (userErr == nil && userReady && dbErr == nil && dbReady) || svc.ResourceExistsInObserved(grantName) {
60+
addGrants(comp, svc, *access.User, dbname, access.Privileges)
61+
} else {
62+
if userErr != nil {
63+
svc.Log.Info("User not yet ready, skipping grant creation", "user", userName, "error", userErr.Error())
64+
}
65+
if dbErr != nil {
66+
svc.Log.Info("Database not yet ready, skipping grant creation", "database", dbName, "error", dbErr.Error())
67+
}
68+
}
4469

4570
addConnectionDetail(comp, svc, userPasswordRef, *access.User, dbname, access.WriteConnectionSecretToReference)
4671
}

‎pkg/comp-functions/functions/vshnmariadb/user_management_test.go‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func Test_addGrants(t *testing.T) {
8686
}
8787

8888
func TestUserManagement(t *testing.T) {
89-
// given with empty accesss object
89+
// given with empty access object
9090
svc := commontest.LoadRuntimeFromFile(t, "vshnmariadb/usermanagement/01-emptyaccess.yaml")
9191

9292
// when applied
@@ -99,7 +99,7 @@ func TestUserManagement(t *testing.T) {
9999
db := &my1alpha1.Database{}
100100
assert.Error(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
101101

102-
// when adding an user
102+
// when adding a user
103103
comp.Spec.Parameters.Service.Access = []vshnv1.VSHNAccess{
104104
{
105105
User: ptr.To("prod"),
@@ -111,9 +111,13 @@ func TestUserManagement(t *testing.T) {
111111
assert.NoError(t, svc.SetDesiredCompositeStatus(comp))
112112
assert.Nil(t, UserManagement(context.TODO(), &vshnv1.VSHNMariaDB{}, svc))
113113

114-
// then expect database
114+
// then expect user to be created but not database yet (sequencing: user must be ready first)
115+
user := &my1alpha1.User{}
116+
assert.NoError(t, svc.GetDesiredComposedResourceByName(user, fmt.Sprintf("%s-%s-role", comp.GetName(), "prod")))
117+
118+
// Database should not be created yet since user is not in observed state
115119
db = &my1alpha1.Database{}
116-
assert.NoError(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
120+
assert.Error(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
117121

118122
// when adding user pointing to same db
119123
comp.Spec.Parameters.Service.Access = append(comp.Spec.Parameters.Service.Access, vshnv1.VSHNAccess{
@@ -126,9 +130,15 @@ func TestUserManagement(t *testing.T) {
126130
assert.NoError(t, svc.SetDesiredCompositeStatus(comp))
127131
assert.Nil(t, UserManagement(context.TODO(), &vshnv1.VSHNMariaDB{}, svc))
128132

129-
// then expect database
133+
// then expect both users but still no database (dependencies not ready)
134+
user = &my1alpha1.User{}
135+
assert.NoError(t, svc.GetDesiredComposedResourceByName(user, fmt.Sprintf("%s-%s-role", comp.GetName(), "prod")))
136+
user2 := &my1alpha1.User{}
137+
assert.NoError(t, svc.GetDesiredComposedResourceByName(user2, fmt.Sprintf("%s-%s-role", comp.GetName(), "another")))
138+
139+
// Database still not created since users are not in observed state
130140
db = &my1alpha1.Database{}
131-
assert.NoError(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
141+
assert.Error(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
132142

133143
// When deleting user pointing to same db
134144
comp.Spec.Parameters.Service.Access = []vshnv1.VSHNAccess{
@@ -140,9 +150,9 @@ func TestUserManagement(t *testing.T) {
140150

141151
assert.NoError(t, setObservedComposition(svc, comp))
142152

143-
// then expect database
144-
db = &my1alpha1.Database{}
145-
assert.NoError(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
153+
// then expect the other user to still be in desired
154+
user = &my1alpha1.User{}
155+
assert.NoError(t, svc.GetDesiredComposedResourceByName(user, fmt.Sprintf("%s-%s-role", comp.GetName(), "another")))
146156
}
147157

148158
func setObservedComposition(svc *runtime.ServiceRuntime, comp *vshnv1.VSHNMariaDB) error {

‎pkg/comp-functions/functions/vshnpostgres/user_management.go‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ func UserManagement(ctx context.Context, comp *vshnv1.VSHNPostgreSQL, svc *runti
3838

3939
for _, access := range comp.Spec.Parameters.Service.Access {
4040

41+
// We don't have a dependency for the user
42+
// Checking for existence of sgcluster does not mean that postgres is ready to accept connections
4143
userPasswordRef := addUser(comp, svc, *access.User)
4244

4345
dbuser := *access.User
@@ -46,9 +48,32 @@ func UserManagement(ctx context.Context, comp *vshnv1.VSHNPostgreSQL, svc *runti
4648
dbname = *access.Database
4749
}
4850

49-
addDatabase(comp, svc, dbuser, dbname)
51+
roleName := fmt.Sprintf("%s-%s-role", comp.GetName(), dbuser)
52+
dbName := fmt.Sprintf("%s-%s-database", comp.GetName(), dbname)
53+
grantName := fmt.Sprintf("%s-%s-%s-grants", comp.GetName(), dbuser, dbname)
54+
55+
// Only add database if role is ready
56+
// We also check for self-existence to prevent deletion when the depending resource turns unready
57+
roleReady, roleErr := svc.IsResourceReady(roleName)
58+
if (roleErr == nil && roleReady) || svc.ResourceExistsInObserved(dbName) {
59+
addDatabase(comp, svc, dbuser, dbname)
60+
} else if roleErr != nil {
61+
svc.Log.Info("Role not yet ready, skipping database creation", "role", roleName, "error", roleErr.Error())
62+
}
5063

51-
addGrants(comp, svc, dbuser, dbname, access.Privileges)
64+
// Only add grants if both role and database are ready
65+
// We also check for self-existence to prevent deletion when the depending resource turns unready
66+
dbReady, dbErr := svc.IsResourceReady(dbName)
67+
if (roleErr == nil && roleReady && dbErr == nil && dbReady) || svc.ResourceExistsInObserved(grantName) {
68+
addGrants(comp, svc, dbuser, dbname, access.Privileges)
69+
} else {
70+
if roleErr != nil {
71+
svc.Log.Info("Role not yet ready, skipping grant creation", "role", roleName, "error", roleErr.Error())
72+
}
73+
if dbErr != nil {
74+
svc.Log.Info("Database not yet ready, skipping grant creation", "database", dbName, "error", dbErr.Error())
75+
}
76+
}
5277

5378
err := addConnectionDetail(comp, svc, userPasswordRef, *access.User, dbname, access.WriteConnectionSecretToReference)
5479
if err != nil {

‎pkg/comp-functions/functions/vshnpostgres/user_management_test.go‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func Test_addGrants(t *testing.T) {
122122
}
123123

124124
func TestUserManagement(t *testing.T) {
125-
// given with empty accesss object
125+
// given with empty access object
126126
svc := commontest.LoadRuntimeFromFile(t, "vshn-postgres/usermanagement/01-emptyaccess.yaml")
127127

128128
// when applied
@@ -135,7 +135,7 @@ func TestUserManagement(t *testing.T) {
135135
db := &pgv1alpha1.Database{}
136136
assert.Error(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
137137

138-
// when adding an user
138+
// when adding a user
139139
comp.Spec.Parameters.Service.Access = []vshnv1.VSHNAccess{
140140
{
141141
User: ptr.To("prod"),
@@ -147,9 +147,13 @@ func TestUserManagement(t *testing.T) {
147147
assert.NoError(t, svc.SetDesiredCompositeStatus(comp))
148148
assert.Nil(t, UserManagement(context.TODO(), &vshnv1.VSHNPostgreSQL{}, svc))
149149

150-
// then expect database
150+
// then expect role to be created but not database yet (sequencing: role must be ready first)
151+
role := &pgv1alpha1.Role{}
152+
assert.NoError(t, svc.GetDesiredComposedResourceByName(role, fmt.Sprintf("%s-%s-role", comp.GetName(), "prod")))
153+
154+
// Database should not be created yet since role is not in observed state
151155
db = &pgv1alpha1.Database{}
152-
assert.NoError(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
156+
assert.Error(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
153157

154158
// when adding user pointing to same db
155159
comp.Spec.Parameters.Service.Access = append(comp.Spec.Parameters.Service.Access, vshnv1.VSHNAccess{
@@ -162,9 +166,15 @@ func TestUserManagement(t *testing.T) {
162166
assert.NoError(t, svc.SetDesiredCompositeStatus(comp))
163167
assert.Nil(t, UserManagement(context.TODO(), &vshnv1.VSHNPostgreSQL{}, svc))
164168

165-
// then expect database
169+
// then expect both roles but still no database (dependencies not ready)
170+
role = &pgv1alpha1.Role{}
171+
assert.NoError(t, svc.GetDesiredComposedResourceByName(role, fmt.Sprintf("%s-%s-role", comp.GetName(), "prod")))
172+
role2 := &pgv1alpha1.Role{}
173+
assert.NoError(t, svc.GetDesiredComposedResourceByName(role2, fmt.Sprintf("%s-%s-role", comp.GetName(), "another")))
174+
175+
// Database still not created since roles are not in observed state
166176
db = &pgv1alpha1.Database{}
167-
assert.NoError(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
177+
assert.Error(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
168178

169179
// When deleting user pointing to same db
170180
comp.Spec.Parameters.Service.Access = []vshnv1.VSHNAccess{
@@ -176,9 +186,9 @@ func TestUserManagement(t *testing.T) {
176186

177187
assert.NoError(t, setObservedComposition(svc, comp))
178188

179-
// then expect database
180-
db = &pgv1alpha1.Database{}
181-
assert.NoError(t, svc.GetDesiredComposedResourceByName(db, fmt.Sprintf("%s-%s-database", comp.GetName(), "prod")))
189+
// then expect the other role to still be in desired
190+
role = &pgv1alpha1.Role{}
191+
assert.NoError(t, svc.GetDesiredComposedResourceByName(role, fmt.Sprintf("%s-%s-role", comp.GetName(), "another")))
182192
}
183193

184194
func setObservedComposition(svc *runtime.ServiceRuntime, comp *vshnv1.VSHNPostgreSQL) error {

‎pkg/comp-functions/runtime/function_mgr.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,3 +1533,38 @@ func (s *ServiceRuntime) CopyKubeResource(ctx context.Context, obj client.Object
15331533

15341534
return instObj, nil
15351535
}
1536+
1537+
// IsResourceReady checks if a resource exists in observed state with Ready=True condition.
1538+
func (s *ServiceRuntime) IsResourceReady(resourceName string) (bool, error) {
1539+
observed, err := s.GetAllObserved()
1540+
if err != nil {
1541+
return false, fmt.Errorf("cannot get observed resources: %w", err)
1542+
}
1543+
1544+
res, ok := observed[resource.Name(resourceName)]
1545+
if !ok {
1546+
return false, fmt.Errorf("resource %s not in observed state", resourceName)
1547+
}
1548+
1549+
cond := res.Resource.GetCondition(xpv1.TypeReady)
1550+
if cond.Status != corev1.ConditionTrue {
1551+
return false, fmt.Errorf("resource %s is not ready: %s", resourceName, cond.Reason)
1552+
}
1553+
1554+
return true, nil
1555+
}
1556+
1557+
// ResourceExistsInObserved checks if a resource exists in observed state.
1558+
// Useful for preventing deletion when dependencies become temporarily unavailable.
1559+
// If GetAllObserved fails, returns true (safer default) and adds a fatal result to halt composition.
1560+
func (s *ServiceRuntime) ResourceExistsInObserved(resourceName string) bool {
1561+
observed, err := s.GetAllObserved()
1562+
if err != nil {
1563+
s.Log.Error(err, "Failed to get observed resources, halting composition", "resourceName", resourceName)
1564+
s.AddResult(NewFatalResult(fmt.Errorf("cannot get observed resources to check if %s exists: %w", resourceName, err)))
1565+
return true
1566+
}
1567+
1568+
_, ok := observed[resource.Name(resourceName)]
1569+
return ok
1570+
}

0 commit comments

Comments
 (0)