Skip to content

Commit 983b239

Browse files
google-oss-prow[bot]google-oss-prow[bot]
authored
Merge pull request GoogleCloudPlatform#2942 from jasonvigil/sqlinstance-defaults
Refactor SQLInstance defaulting
2 parents 40d1120 + cee5d87 commit 983b239

File tree

32 files changed

+172
-50
lines changed

32 files changed

+172
-50
lines changed

‎pkg/controller/direct/sql/sqlinstance_controller.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func (m *sqlInstanceModel) AdapterForObject(ctx context.Context, kube client.Rea
108108
return nil, fmt.Errorf("building gcp client: %w", err)
109109
}
110110

111-
if err := NormalizeSQLInstance(ctx, kube, obj); err != nil {
111+
if err := ResolveSQLInstanceRefs(ctx, kube, obj); err != nil {
112112
return nil, err
113113
}
114114

‎pkg/controller/direct/sql/sqlinstance_defaults.go

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2024 Google LLC
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 sql
16+
17+
import (
18+
krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/sql/v1beta1"
19+
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct"
20+
api "google.golang.org/api/sqladmin/v1beta4"
21+
)
22+
23+
func ApplySQLInstanceGCPDefaults(in *krm.SQLInstance, out *api.DatabaseInstance) {
24+
if in.Spec.InstanceType == nil {
25+
out.InstanceType = "CLOUD_SQL_INSTANCE"
26+
}
27+
if in.Spec.Settings.ActivationPolicy == nil {
28+
out.Settings.ActivationPolicy = "ALWAYS"
29+
}
30+
if in.Spec.Settings.AvailabilityType == nil {
31+
out.Settings.AvailabilityType = "ZONAL"
32+
}
33+
if in.Spec.Settings.ConnectorEnforcement == nil {
34+
out.Settings.ConnectorEnforcement = "NOT_REQUIRED"
35+
}
36+
if in.Spec.Settings.DiskType == nil {
37+
out.Settings.DataDiskType = "PD_SSD"
38+
}
39+
if in.Spec.Settings.Edition == nil {
40+
out.Settings.Edition = "ENTERPRISE"
41+
}
42+
if in.Spec.Settings.PricingPlan == nil {
43+
out.Settings.PricingPlan = "PER_USE"
44+
}
45+
if in.Spec.Settings.DiskAutoresize == nil {
46+
out.Settings.StorageAutoResize = direct.PtrTo(true)
47+
}
48+
}

‎pkg/controller/direct/sql/sqlinstance_mappings.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
2424
krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/sql/v1beta1"
2525
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct"
26+
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/label"
2627
)
2728

2829
func SQLInstanceKRMToGCP(in *krm.SQLInstance) (*api.DatabaseInstance, error) {
@@ -50,6 +51,9 @@ func SQLInstanceKRMToGCP(in *krm.SQLInstance) (*api.DatabaseInstance, error) {
5051
// SwitchTransactionLogsToCloudStorageEnabled is not supported in KRM API.
5152
}
5253

54+
// Here be dragons.
55+
ApplySQLInstanceGCPDefaults(in, out)
56+
5357
return out, nil
5458
}
5559

@@ -140,7 +144,7 @@ func InstanceSettingsKRMToGCP(in krm.InstanceSettings, labels map[string]string)
140144
StorageAutoResizeLimit: direct.ValueOf(in.DiskAutoresizeLimit),
141145
Tier: in.Tier,
142146
TimeZone: direct.ValueOf(in.TimeZone),
143-
UserLabels: labels,
147+
UserLabels: label.NewGCPLabelsFromK8sLabels(labels),
144148
}
145149

146150
if in.CrashSafeReplication != nil {

‎pkg/controller/direct/sql/sqlinstance_normalize.go ‎pkg/controller/direct/sql/sqlinstance_resolverefs.go

+15-48
Original file line numberDiff line numberDiff line change
@@ -24,50 +24,39 @@ import (
2424
storagev1beta1 "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/clients/generated/apis/storage/v1beta1"
2525
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct"
2626
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/k8s"
27-
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/label"
2827
corev1 "k8s.io/api/core/v1"
2928
apierrors "k8s.io/apimachinery/pkg/api/errors"
3029
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3130
"k8s.io/apimachinery/pkg/types"
3231
"sigs.k8s.io/controller-runtime/pkg/client"
3332
)
3433

35-
func NormalizeSQLInstance(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
36-
// Apply defaults.
37-
if err := normalizeTFDefaults(obj); err != nil {
34+
func ResolveSQLInstanceRefs(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
35+
if err := resolveCryptoKeyRef(ctx, kube, obj); err != nil {
3836
return err
3937
}
40-
41-
// Normalize references.
42-
if err := normalizeCryptoKeyRef(ctx, kube, obj); err != nil {
43-
return err
44-
}
45-
if err := normalizeMasterInstanceRef(ctx, kube, obj); err != nil {
38+
if err := resolveMasterInstanceRef(ctx, kube, obj); err != nil {
4639
return err
4740
}
48-
if err := normalizeReplicaPasswordRef(ctx, kube, obj); err != nil {
41+
if err := resolveReplicaPasswordRef(ctx, kube, obj); err != nil {
4942
return err
5043
}
51-
if err := normalizeRootPasswordRef(ctx, kube, obj); err != nil {
44+
if err := resolveRootPasswordRef(ctx, kube, obj); err != nil {
5245
return err
5346
}
54-
if err := normalizePrivateNetworkRef(ctx, kube, obj); err != nil {
47+
if err := resolvePrivateNetworkRef(ctx, kube, obj); err != nil {
5548
return err
5649
}
57-
if err := normalizeAuditLogBucketRef(ctx, kube, obj); err != nil {
50+
if err := resolveAuditLogBucketRef(ctx, kube, obj); err != nil {
5851
return err
5952
}
60-
if err := normalizeSourceSQLInstanceRef(ctx, kube, obj); err != nil {
53+
if err := resolveSourceSQLInstanceRef(ctx, kube, obj); err != nil {
6154
return err
6255
}
63-
64-
// Filter labels.
65-
obj.Labels = label.NewGCPLabelsFromK8sLabels(obj.Labels)
66-
6756
return nil
6857
}
6958

70-
func normalizeCryptoKeyRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
59+
func resolveCryptoKeyRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
7160
if obj.Spec.EncryptionKMSCryptoKeyRef == nil {
7261
return nil
7362
}
@@ -112,7 +101,7 @@ func normalizeCryptoKeyRef(ctx context.Context, kube client.Reader, obj *krm.SQL
112101
}
113102
}
114103

115-
func normalizeMasterInstanceRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
104+
func resolveMasterInstanceRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
116105
if obj.Spec.MasterInstanceRef == nil {
117106
return nil
118107
}
@@ -155,7 +144,7 @@ func normalizeMasterInstanceRef(ctx context.Context, kube client.Reader, obj *kr
155144
}
156145
}
157146

158-
func normalizeReplicaPasswordRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
147+
func resolveReplicaPasswordRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
159148
if obj.Spec.ReplicaConfiguration == nil || obj.Spec.ReplicaConfiguration.Password == nil {
160149
return nil
161150
}
@@ -189,7 +178,7 @@ func normalizeReplicaPasswordRef(ctx context.Context, kube client.Reader, obj *k
189178
return nil
190179
}
191180

192-
func normalizeRootPasswordRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
181+
func resolveRootPasswordRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
193182
if obj.Spec.RootPassword == nil {
194183
return nil
195184
}
@@ -223,7 +212,7 @@ func normalizeRootPasswordRef(ctx context.Context, kube client.Reader, obj *krm.
223212
return nil
224213
}
225214

226-
func normalizePrivateNetworkRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
215+
func resolvePrivateNetworkRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
227216
if obj.Spec.Settings.IpConfiguration == nil || obj.Spec.Settings.IpConfiguration.PrivateNetworkRef == nil {
228217
return nil
229218
}
@@ -244,7 +233,7 @@ func normalizePrivateNetworkRef(ctx context.Context, kube client.Reader, obj *kr
244233
return nil
245234
}
246235

247-
func normalizeAuditLogBucketRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
236+
func resolveAuditLogBucketRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
248237
if obj.Spec.Settings.SqlServerAuditConfig == nil {
249238
return nil
250239
}
@@ -292,7 +281,7 @@ func normalizeAuditLogBucketRef(ctx context.Context, kube client.Reader, obj *kr
292281
}
293282
}
294283

295-
func normalizeSourceSQLInstanceRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
284+
func resolveSourceSQLInstanceRef(ctx context.Context, kube client.Reader, obj *krm.SQLInstance) error {
296285
if obj.Spec.CloneSource == nil {
297286
return nil
298287
}
@@ -336,25 +325,3 @@ func normalizeSourceSQLInstanceRef(ctx context.Context, kube client.Reader, obj
336325
return fmt.Errorf("must specify either spec.settings.cloneSource.sqlInstanceRef.external or spec.settings.cloneSource.sqlInstanceRef.name")
337326
}
338327
}
339-
340-
func normalizeTFDefaults(obj *krm.SQLInstance) error {
341-
if obj.Spec.Settings.ActivationPolicy == nil {
342-
obj.Spec.Settings.ActivationPolicy = direct.PtrTo("ALWAYS")
343-
}
344-
if obj.Spec.Settings.AvailabilityType == nil {
345-
obj.Spec.Settings.AvailabilityType = direct.PtrTo("ZONAL")
346-
}
347-
if obj.Spec.Settings.DiskType == nil {
348-
obj.Spec.Settings.DiskType = direct.PtrTo("PD_SSD")
349-
}
350-
if obj.Spec.Settings.Edition == nil {
351-
obj.Spec.Settings.Edition = direct.PtrTo("ENTERPRISE")
352-
}
353-
if obj.Spec.Settings.PricingPlan == nil {
354-
obj.Spec.Settings.PricingPlan = direct.PtrTo("PER_USE")
355-
}
356-
if obj.Spec.Settings.DiskAutoresize == nil {
357-
obj.Spec.Settings.DiskAutoresize = direct.PtrTo(true)
358-
}
359-
return nil
360-
}

‎pkg/test/resourcefixture/testdata/basic/sql/v1beta1/sqlinstance/sqlinstance-activationpolicy-direct/_http.log

+4
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,14 @@ User-Agent: kcc/controller-manager
3434

3535
{
3636
"databaseVersion": "POSTGRES_15",
37+
"instanceType": "CLOUD_SQL_INSTANCE",
3738
"kind": "sql#instance",
3839
"name": "sqlinstance-activationpolicy-direct-${uniqueId}",
3940
"region": "us-central1",
4041
"settings": {
4142
"activationPolicy": "ALWAYS",
4243
"availabilityType": "ZONAL",
44+
"connectorEnforcement": "NOT_REQUIRED",
4345
"dataDiskType": "PD_SSD",
4446
"edition": "ENTERPRISE",
4547
"kind": "sql#settings",
@@ -374,12 +376,14 @@ User-Agent: kcc/controller-manager
374376

375377
{
376378
"databaseVersion": "POSTGRES_15",
379+
"instanceType": "CLOUD_SQL_INSTANCE",
377380
"kind": "sql#instance",
378381
"name": "sqlinstance-activationpolicy-direct-${uniqueId}",
379382
"region": "us-central1",
380383
"settings": {
381384
"activationPolicy": "NEVER",
382385
"availabilityType": "ZONAL",
386+
"connectorEnforcement": "NOT_REQUIRED",
383387
"dataDiskType": "PD_SSD",
384388
"edition": "ENTERPRISE",
385389
"kind": "sql#settings",

‎pkg/test/resourcefixture/testdata/basic/sql/v1beta1/sqlinstance/sqlinstance-auditconfig-direct/_http.log

+4
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,15 @@ User-Agent: kcc/controller-manager
173173

174174
{
175175
"databaseVersion": "SQLSERVER_2022_EXPRESS",
176+
"instanceType": "CLOUD_SQL_INSTANCE",
176177
"kind": "sql#instance",
177178
"name": "sqlinstance-auditconfig-direct-${uniqueId}",
178179
"region": "us-central1",
179180
"rootPassword": "1234!@#$asdf",
180181
"settings": {
181182
"activationPolicy": "ALWAYS",
182183
"availabilityType": "ZONAL",
184+
"connectorEnforcement": "NOT_REQUIRED",
183185
"dataDiskType": "PD_SSD",
184186
"edition": "ENTERPRISE",
185187
"kind": "sql#settings",
@@ -557,13 +559,15 @@ User-Agent: kcc/controller-manager
557559

558560
{
559561
"databaseVersion": "SQLSERVER_2022_EXPRESS",
562+
"instanceType": "CLOUD_SQL_INSTANCE",
560563
"kind": "sql#instance",
561564
"name": "sqlinstance-auditconfig-direct-${uniqueId}",
562565
"region": "us-central1",
563566
"rootPassword": "1234!@#$asdf",
564567
"settings": {
565568
"activationPolicy": "ALWAYS",
566569
"availabilityType": "ZONAL",
570+
"connectorEnforcement": "NOT_REQUIRED",
567571
"dataDiskType": "PD_SSD",
568572
"edition": "ENTERPRISE",
569573
"kind": "sql#settings",

‎pkg/test/resourcefixture/testdata/basic/sql/v1beta1/sqlinstance/sqlinstance-authorizednetworks-direct/_http.log

+4
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,14 @@ User-Agent: kcc/controller-manager
3434

3535
{
3636
"databaseVersion": "POSTGRES_16",
37+
"instanceType": "CLOUD_SQL_INSTANCE",
3738
"kind": "sql#instance",
3839
"name": "sqlinstance-authorizednetworks-direct-${uniqueId}",
3940
"region": "us-central1",
4041
"settings": {
4142
"activationPolicy": "ALWAYS",
4243
"availabilityType": "ZONAL",
44+
"connectorEnforcement": "NOT_REQUIRED",
4345
"dataDiskType": "PD_SSD",
4446
"edition": "ENTERPRISE",
4547
"ipConfiguration": {
@@ -370,12 +372,14 @@ User-Agent: kcc/controller-manager
370372

371373
{
372374
"databaseVersion": "POSTGRES_16",
375+
"instanceType": "CLOUD_SQL_INSTANCE",
373376
"kind": "sql#instance",
374377
"name": "sqlinstance-authorizednetworks-direct-${uniqueId}",
375378
"region": "us-central1",
376379
"settings": {
377380
"activationPolicy": "ALWAYS",
378381
"availabilityType": "ZONAL",
382+
"connectorEnforcement": "NOT_REQUIRED",
379383
"dataDiskType": "PD_SSD",
380384
"edition": "ENTERPRISE",
381385
"ipConfiguration": {

‎pkg/test/resourcefixture/testdata/basic/sql/v1beta1/sqlinstance/sqlinstance-backupconfiguration-binarylog-direct/_http.log

+4
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ User-Agent: kcc/controller-manager
3434

3535
{
3636
"databaseVersion": "MYSQL_5_7",
37+
"instanceType": "CLOUD_SQL_INSTANCE",
3738
"kind": "sql#instance",
3839
"name": "sqlinstance-backupconfiguration-binarylog-direct-${uniqueId}",
3940
"region": "us-central1",
@@ -53,6 +54,7 @@ User-Agent: kcc/controller-manager
5354
"startTime": "12:00",
5455
"transactionLogRetentionDays": 2
5556
},
57+
"connectorEnforcement": "NOT_REQUIRED",
5658
"dataDiskType": "PD_SSD",
5759
"edition": "ENTERPRISE",
5860
"kind": "sql#settings",
@@ -569,6 +571,7 @@ User-Agent: kcc/controller-manager
569571

570572
{
571573
"databaseVersion": "MYSQL_5_7",
574+
"instanceType": "CLOUD_SQL_INSTANCE",
572575
"kind": "sql#instance",
573576
"name": "sqlinstance-backupconfiguration-binarylog-direct-${uniqueId}",
574577
"region": "us-central1",
@@ -588,6 +591,7 @@ User-Agent: kcc/controller-manager
588591
"startTime": "12:00",
589592
"transactionLogRetentionDays": 3
590593
},
594+
"connectorEnforcement": "NOT_REQUIRED",
591595
"dataDiskType": "PD_SSD",
592596
"edition": "ENTERPRISE",
593597
"kind": "sql#settings",

‎pkg/test/resourcefixture/testdata/basic/sql/v1beta1/sqlinstance/sqlinstance-backupconfiguration-pitr-direct/_http.log

+4
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ User-Agent: kcc/controller-manager
3434

3535
{
3636
"databaseVersion": "POSTGRES_15",
37+
"instanceType": "CLOUD_SQL_INSTANCE",
3738
"kind": "sql#instance",
3839
"name": "sqlinstance-backupconfiguration-pitr-direct-${uniqueId}",
3940
"region": "us-central1",
@@ -53,6 +54,7 @@ User-Agent: kcc/controller-manager
5354
"startTime": "12:00",
5455
"transactionLogRetentionDays": 2
5556
},
57+
"connectorEnforcement": "NOT_REQUIRED",
5658
"dataDiskType": "PD_SSD",
5759
"edition": "ENTERPRISE",
5860
"kind": "sql#settings",
@@ -393,6 +395,7 @@ User-Agent: kcc/controller-manager
393395

394396
{
395397
"databaseVersion": "POSTGRES_15",
398+
"instanceType": "CLOUD_SQL_INSTANCE",
396399
"kind": "sql#instance",
397400
"name": "sqlinstance-backupconfiguration-pitr-direct-${uniqueId}",
398401
"region": "us-central1",
@@ -412,6 +415,7 @@ User-Agent: kcc/controller-manager
412415
"startTime": "12:00",
413416
"transactionLogRetentionDays": 3
414417
},
418+
"connectorEnforcement": "NOT_REQUIRED",
415419
"dataDiskType": "PD_SSD",
416420
"edition": "ENTERPRISE",
417421
"kind": "sql#settings",

‎pkg/test/resourcefixture/testdata/basic/sql/v1beta1/sqlinstance/sqlinstance-clone-minimal-direct/_http.log

+2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ User-Agent: kcc/controller-manager
3434

3535
{
3636
"databaseVersion": "POSTGRES_16",
37+
"instanceType": "CLOUD_SQL_INSTANCE",
3738
"kind": "sql#instance",
3839
"name": "sqlinstance-source-direct-${uniqueId}",
3940
"region": "us-central1",
@@ -45,6 +46,7 @@ User-Agent: kcc/controller-manager
4546
"kind": "sql#backupConfiguration",
4647
"pointInTimeRecoveryEnabled": true
4748
},
49+
"connectorEnforcement": "NOT_REQUIRED",
4850
"dataDiskType": "PD_SSD",
4951
"edition": "ENTERPRISE",
5052
"kind": "sql#settings",

0 commit comments

Comments
 (0)