diff --git a/api/v1beta1/azuresqldatabase_types.go b/api/v1beta1/azuresqldatabase_types.go index aba5bdd0088..1497f495f49 100644 --- a/api/v1beta1/azuresqldatabase_types.go +++ b/api/v1beta1/azuresqldatabase_types.go @@ -40,6 +40,13 @@ type SqlDatabaseSku struct { Capacity *int32 `json:"capacity,omitempty"` } +type SQLDatabaseShortTermRetentionPolicy struct { + // RetentionDays is the backup retention period in days. This is how many days + // Point-in-Time Restore will be supported. + // +kubebuilder:validation:Required + RetentionDays int32 `json:"retentionDays"` +} + // AzureSqlDatabaseSpec defines the desired state of AzureSqlDatabase type AzureSqlDatabaseSpec struct { // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster @@ -59,14 +66,15 @@ type AzureSqlDatabaseSpec struct { Server string `json:"server"` // +kubebuilder:validation:Optional - Edition DBEdition `json:"edition"` // TODO: Remove this in v1beta2 - Sku *SqlDatabaseSku `json:"sku,omitempty"` // TODO: make this required in v1beta2 - MaxSize *resource.Quantity `json:"maxSize,omitempty"` - DbName string `json:"dbName,omitempty"` - WeeklyRetention string `json:"weeklyRetention,omitempty"` - MonthlyRetention string `json:"monthlyRetention,omitempty"` - YearlyRetention string `json:"yearlyRetention,omitempty"` - WeekOfYear int32 `json:"weekOfYear,omitempty"` + Edition DBEdition `json:"edition"` // TODO: Remove this in v1beta2 + Sku *SqlDatabaseSku `json:"sku,omitempty"` // TODO: make this required in v1beta2 + MaxSize *resource.Quantity `json:"maxSize,omitempty"` + DbName string `json:"dbName,omitempty"` + WeeklyRetention string `json:"weeklyRetention,omitempty"` + MonthlyRetention string `json:"monthlyRetention,omitempty"` + YearlyRetention string `json:"yearlyRetention,omitempty"` + WeekOfYear int32 `json:"weekOfYear,omitempty"` + ShortTermRetentionPolicy *SQLDatabaseShortTermRetentionPolicy `json:"shortTermRetentionPolicy,omitempty"` } // AzureSqlDatabase is the Schema for the azuresqldatabases API diff --git a/config/samples/azure_v1beta1_azuresqldatabase.yaml b/config/samples/azure_v1beta1_azuresqldatabase.yaml index b107b166cf7..487a1873f3b 100644 --- a/config/samples/azure_v1beta1_azuresqldatabase.yaml +++ b/config/samples/azure_v1beta1_azuresqldatabase.yaml @@ -28,4 +28,9 @@ spec: # The week of year to take the yearly backup, valid values [1, 52] # weekOfYear: 16 - \ No newline at end of file + + # The short term retention policy to use + # shortTermRetentionPolicy: + # RetentionDays is the backup retention period in days. This is how many days + # Point-in-Time Restore will be supported. + # retentionDays: 21 diff --git a/controllers/azuresql_combined_test.go b/controllers/azuresql_combined_test.go index e3a9380620b..9a2953fd056 100644 --- a/controllers/azuresql_combined_test.go +++ b/controllers/azuresql_combined_test.go @@ -11,17 +11,18 @@ import ( "strings" "testing" - azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" - "github.com/Azure/azure-service-operator/api/v1beta1" "github.com/stretchr/testify/assert" - - helpers "github.com/Azure/azure-service-operator/pkg/helpers" - "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" - kvsecrets "github.com/Azure/azure-service-operator/pkg/secrets/keyvault" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + + azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" + "github.com/Azure/azure-service-operator/api/v1beta1" + "github.com/Azure/azure-service-operator/pkg/errhelp" + helpers "github.com/Azure/azure-service-operator/pkg/helpers" + "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" + kvsecrets "github.com/Azure/azure-service-operator/pkg/secrets/keyvault" ) func TestAzureSqlServerCombinedHappyPath(t *testing.T) { @@ -65,8 +66,10 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) { sqlDatabaseName1 := GenerateTestResourceNameWithRandom("sqldatabase", 10) sqlDatabaseName2 := GenerateTestResourceNameWithRandom("sqldatabase", 10) + sqlDatabaseName3 := GenerateTestResourceNameWithRandom("sqldatabase", 10) var sqlDatabaseInstance1 *v1beta1.AzureSqlDatabase var sqlDatabaseInstance2 *v1beta1.AzureSqlDatabase + var sqlDatabaseInstance3 *v1beta1.AzureSqlDatabase sqlFirewallRuleNamespacedNameLocal := types.NamespacedName{ Name: GenerateTestResourceNameWithRandom("sqlfwr-local", 10), @@ -190,6 +193,47 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) { assert.Equal(true, db.Status.Provisioned) }) + // Create a database in the new server + t.Run("set up database with short and long term retention", func(t *testing.T) { + t.Parallel() + + // Create the SqlDatabase object and expect the Reconcile to be created + sqlDatabaseInstance3 = &v1beta1.AzureSqlDatabase{ + ObjectMeta: metav1.ObjectMeta{ + Name: sqlDatabaseName3, + Namespace: "default", + }, + Spec: v1beta1.AzureSqlDatabaseSpec{ + Location: rgLocation, + ResourceGroup: rgName, + Server: sqlServerName, + Sku: &v1beta1.SqlDatabaseSku{ + Name: "S0", + Tier: "Standard", + }, + WeeklyRetention: "P3W", + ShortTermRetentionPolicy: &v1beta1.SQLDatabaseShortTermRetentionPolicy{ + RetentionDays: 3, + }, + }, + } + + EnsureInstance(ctx, t, tc, sqlDatabaseInstance3) + + // Now update with an invalid retention policy + sqlDatabaseInstance3.Spec.ShortTermRetentionPolicy.RetentionDays = -1 + err = tc.k8sClient.Update(ctx, sqlDatabaseInstance3) + assert.Equal(nil, err, "updating sql database in k8s") + + namespacedName := types.NamespacedName{Name: sqlDatabaseName3, Namespace: "default"} + assert.Eventually(func() bool { + db := &v1beta1.AzureSqlDatabase{} + err = tc.k8sClient.Get(ctx, namespacedName, db) + assert.Equal(nil, err, "err getting DB from k8s") + return db.Status.Provisioned == false && strings.Contains(db.Status.Message, errhelp.BackupRetentionPolicyInvalid) + }, tc.timeout, tc.retry, "wait for sql database to be updated in k8s") + }) + // Create FirewallRules --------------------------------------- t.Run("set up wide range firewall rule in primary server", func(t *testing.T) { @@ -494,6 +538,7 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) { t.Parallel() EnsureDelete(ctx, t, tc, sqlDatabaseInstance1) EnsureDelete(ctx, t, tc, sqlDatabaseInstance2) + EnsureDelete(ctx, t, tc, sqlDatabaseInstance3) }) }) diff --git a/controllers/keyvault_controller_test.go b/controllers/keyvault_controller_test.go index 32d132a1703..e7ea31c4553 100644 --- a/controllers/keyvault_controller_test.go +++ b/controllers/keyvault_controller_test.go @@ -407,8 +407,8 @@ func TestKeyvaultControllerBadAccessPolicy(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.KeyVaultSpec{ - Location: keyVaultLocation, - ResourceGroup: tc.resourceGroupName, + Location: keyVaultLocation, + ResourceGroup: tc.resourceGroupName, AccessPolicies: &accessPolicies, }, } diff --git a/pkg/errhelp/errors.go b/pkg/errhelp/errors.go index 0b0dcacee2f..ebe96f6551e 100644 --- a/pkg/errhelp/errors.go +++ b/pkg/errhelp/errors.go @@ -66,6 +66,7 @@ const ( FeatureNotSupportedForEdition = "FeatureNotSupportedForEdition" VirtualNetworkRuleBadRequest = "VirtualNetworkRuleBadRequest" LongTermRetentionPolicyInvalid = "LongTermRetentionPolicyInvalid" + BackupRetentionPolicyInvalid = "InvalidBackupRetentionPeriod" OperationIdNotFound = "OperationIdNotFound" ) diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb.go index 322017256c6..6990521cdf3 100644 --- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb.go +++ b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb.go @@ -6,10 +6,11 @@ package azuresqldb import ( "context" "fmt" - "net/http" "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql" - sql3 "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql" + "github.com/pkg/errors" + + "github.com/Azure/azure-service-operator/api/v1beta1" "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/helpers" azuresqlshared "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared" @@ -139,42 +140,40 @@ func (m *AzureSqlDbManager) CreateOrUpdateDB( } // AddLongTermRetention enables / disables long term retention -func (m *AzureSqlDbManager) AddLongTermRetention(ctx context.Context, resourceGroupName string, serverName string, databaseName string, weeklyRetention string, monthlyRetention string, yearlyRetention string, weekOfYear int32) (*http.Response, error) { +func (m *AzureSqlDbManager) AddLongTermRetention( + ctx context.Context, + resourceGroupName string, + serverName string, + databaseName string, + policy azuresqlshared.SQLDatabaseBackupLongTermRetentionPolicy) (*sql.BackupLongTermRetentionPoliciesCreateOrUpdateFuture, error) { longTermClient, err := azuresqlshared.GetBackupLongTermRetentionPoliciesClient(m.creds) - // TODO: Probably shouldn't return a response at all in the err case here (all through this function) if err != nil { - return &http.Response{ - StatusCode: 0, - }, err + return nil, err } // validate the input and exit if nothing needs to happen - this is ok! - if weeklyRetention == "" && monthlyRetention == "" && yearlyRetention == "" { - return &http.Response{ - StatusCode: 200, - }, nil + if policy.WeeklyRetention == "" && policy.MonthlyRetention == "" && policy.YearlyRetention == "" { + return nil, nil } // validate the pairing of yearly retention and week of year - if yearlyRetention != "" && (weekOfYear <= 0 || weekOfYear > 52) { - return &http.Response{ - StatusCode: 500, - }, fmt.Errorf("weekOfYear must be greater than 0 and less or equal to 52 when yearlyRetention is used") + if policy.YearlyRetention != "" && (policy.WeekOfYear <= 0 || policy.WeekOfYear > 52) { + return nil, fmt.Errorf("weekOfYear must be greater than 0 and less or equal to 52 when yearlyRetention is used") } // create pointers so that we can pass nils if needed - pWeeklyRetention := &weeklyRetention - if weeklyRetention == "" { + pWeeklyRetention := &policy.WeeklyRetention + if policy.WeeklyRetention == "" { pWeeklyRetention = nil } - pMonthlyRetention := &monthlyRetention - if monthlyRetention == "" { + pMonthlyRetention := &policy.MonthlyRetention + if policy.MonthlyRetention == "" { pMonthlyRetention = nil } - pYearlyRetention := &yearlyRetention - pWeekOfYear := &weekOfYear - if yearlyRetention == "" { + pYearlyRetention := &policy.YearlyRetention + pWeekOfYear := &policy.WeekOfYear + if policy.YearlyRetention == "" { pYearlyRetention = nil pWeekOfYear = nil } @@ -184,8 +183,8 @@ func (m *AzureSqlDbManager) AddLongTermRetention(ctx context.Context, resourceGr resourceGroupName, serverName, databaseName, - sql3.BackupLongTermRetentionPolicy{ - LongTermRetentionPolicyProperties: &sql3.LongTermRetentionPolicyProperties{ + sql.BackupLongTermRetentionPolicy{ + LongTermRetentionPolicyProperties: &sql.LongTermRetentionPolicyProperties{ WeeklyRetention: pWeeklyRetention, MonthlyRetention: pMonthlyRetention, YearlyRetention: pYearlyRetention, @@ -195,12 +194,57 @@ func (m *AzureSqlDbManager) AddLongTermRetention(ctx context.Context, resourceGr ) if err != nil { - return &http.Response{ - StatusCode: 500, - }, nil + return nil, err + } + + return &future, err +} + +func (m *AzureSqlDbManager) AddShortTermRetention( + ctx context.Context, + resourceGroupName string, + serverName string, + databaseName string, + policy *v1beta1.SQLDatabaseShortTermRetentionPolicy) (*sql.BackupShortTermRetentionPoliciesCreateOrUpdateFuture, error) { + + client, err := azuresqlshared.GetBackupShortTermRetentionPoliciesClient(m.creds) + if err != nil { + return nil, errors.Wrapf(err, "couldn't create BackupShortTermRetentionPoliciesClient") + } + + var policyProperties *sql.BackupShortTermRetentionPolicyProperties + if policy == nil { + // If policy is nil we're in a bit of an awkward situation since we cannot know if the customer has mutated + // the retention policy in a previous reconciliation loop and then subsequently removed it. If they have, + // "doing nothing" here is wrong because that leaves them in the previous modified state (but with no reflection + // of that fact in the Spec). + // Unfortunately you cannot update the retention policy to nil, nor can you delete it, so we must awkwardly + // set it back to its default configuration. + // Note: There are risks here, such as if the default on the server and the default in our code drift apart + // at some point in the future. + policyProperties = &sql.BackupShortTermRetentionPolicyProperties{ + RetentionDays: to.Int32Ptr(7), // 7 is the magical default as of Jan 2021 + } + } else { + policyProperties = &sql.BackupShortTermRetentionPolicyProperties{ + RetentionDays: to.Int32Ptr(policy.RetentionDays), + } + } + + future, err := client.CreateOrUpdate( + ctx, + resourceGroupName, + serverName, + databaseName, + sql.BackupShortTermRetentionPolicy{ + BackupShortTermRetentionPolicyProperties: policyProperties, + }) + + if err != nil { + return nil, err } - return future.Response(), err + return &future, err } var goneCodes = []string{ diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_manager.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_manager.go index 6aa078072bd..f78ffa98125 100644 --- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_manager.go +++ b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_manager.go @@ -5,12 +5,11 @@ package azuresqldb import ( "context" - "net/http" "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql" - azuresqlshared "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared" "github.com/Azure/azure-service-operator/pkg/resourcemanager" + "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared" ) // SqlDbManager is the client for the resource manager for SQL databases @@ -39,10 +38,7 @@ type SqlDbManager interface { resourceGroupName string, serverName string, databaseName string, - weeklyRetention string, - monthlyRetention string, - yearlyRetention string, - weekOfYear int32) (*http.Response, error) + policy azuresqlshared.SQLDatabaseBackupLongTermRetentionPolicy) (*sql.BackupLongTermRetentionPoliciesCreateOrUpdateFuture, error) resourcemanager.ARMClient } diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go index 330e3d073ae..f9a878c7f26 100644 --- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go @@ -114,10 +114,12 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt groupName, server, dbName, - instance.Spec.WeeklyRetention, - instance.Spec.MonthlyRetention, - instance.Spec.YearlyRetention, - instance.Spec.WeekOfYear) + azuresqlshared.SQLDatabaseBackupLongTermRetentionPolicy{ + WeeklyRetention: instance.Spec.WeeklyRetention, + MonthlyRetention: instance.Spec.MonthlyRetention, + YearlyRetention: instance.Spec.YearlyRetention, + WeekOfYear: instance.Spec.WeekOfYear, + }) if err != nil { failureErrors := []string{ errhelp.LongTermRetentionPolicyInvalid, @@ -133,6 +135,27 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt } } + _, err = db.AddShortTermRetention( + ctx, + groupName, + server, + dbName, + instance.Spec.ShortTermRetentionPolicy) + if err != nil { + failureErrors := []string{ + errhelp.BackupRetentionPolicyInvalid, + } + instance.Status.Message = fmt.Sprintf("Azure DB short-term retention policy error: %s", errhelp.StripErrorIDs(err)) + azerr := errhelp.NewAzureError(err) + if helpers.ContainsString(failureErrors, azerr.Type) { + // Leave message the same as above + instance.Status.SetFailedProvisioning(instance.Status.Message) + return true, nil + } else { + return false, err + } + } + // db exists, we have successfully provisioned everything instance.Status.SetProvisioned(resourcemanager.SuccessMsg) instance.Status.State = string(dbGet.Status) diff --git a/pkg/resourcemanager/azuresql/azuresqlshared/getgoclients.go b/pkg/resourcemanager/azuresql/azuresqlshared/getgoclients.go index ec6db332bf0..654aac20f83 100644 --- a/pkg/resourcemanager/azuresql/azuresqlshared/getgoclients.go +++ b/pkg/resourcemanager/azuresql/azuresqlshared/getgoclients.go @@ -61,36 +61,48 @@ func GetGoFirewallClient(creds config.Credentials) (sql.FirewallRulesClient, err // GetGoVNetRulesClient retrieves a VirtualNetworkRulesClient func GetGoVNetRulesClient(creds config.Credentials) (sql.VirtualNetworkRulesClient, error) { - VNetRulesClient := sql.NewVirtualNetworkRulesClientWithBaseURI(config.BaseURI(), creds.SubscriptionID()) + vnetRulesClient := sql.NewVirtualNetworkRulesClientWithBaseURI(config.BaseURI(), creds.SubscriptionID()) a, err := iam.GetResourceManagementAuthorizer(creds) if err != nil { return sql.VirtualNetworkRulesClient{}, err } - VNetRulesClient.Authorizer = a - VNetRulesClient.AddToUserAgent(config.UserAgent()) - return VNetRulesClient, nil + vnetRulesClient.Authorizer = a + vnetRulesClient.AddToUserAgent(config.UserAgent()) + return vnetRulesClient, nil } // GetNetworkSubnetClient retrieves a Subnetclient func GetGoNetworkSubnetClient(creds config.Credentials, subscription string) (network.SubnetsClient, error) { - SubnetsClient := network.NewSubnetsClientWithBaseURI(config.BaseURI(), subscription) + subnetsClient := network.NewSubnetsClientWithBaseURI(config.BaseURI(), subscription) a, err := iam.GetResourceManagementAuthorizer(creds) if err != nil { return network.SubnetsClient{}, err } - SubnetsClient.Authorizer = a - SubnetsClient.AddToUserAgent(config.UserAgent()) - return SubnetsClient, nil + subnetsClient.Authorizer = a + subnetsClient.AddToUserAgent(config.UserAgent()) + return subnetsClient, nil } -// GetBackupLongTermRetentionPoliciesClient retrieves a Subnetclient +// GetBackupLongTermRetentionPoliciesClient retrieves a BackupLongTermRetentionPoliciesClient func GetBackupLongTermRetentionPoliciesClient(creds config.Credentials) (sql3.BackupLongTermRetentionPoliciesClient, error) { - BackupClient := sql3.NewBackupLongTermRetentionPoliciesClientWithBaseURI(config.BaseURI(), creds.SubscriptionID()) + backupClient := sql3.NewBackupLongTermRetentionPoliciesClientWithBaseURI(config.BaseURI(), creds.SubscriptionID()) a, err := iam.GetResourceManagementAuthorizer(creds) if err != nil { return sql3.BackupLongTermRetentionPoliciesClient{}, err } - BackupClient.Authorizer = a - BackupClient.AddToUserAgent(config.UserAgent()) - return BackupClient, nil + backupClient.Authorizer = a + backupClient.AddToUserAgent(config.UserAgent()) + return backupClient, nil +} + +// GetBackupShortTermRetentionPoliciesClient retrieves a BackupShortTermRetentionPoliciesClient +func GetBackupShortTermRetentionPoliciesClient(creds config.Credentials) (sql3.BackupShortTermRetentionPoliciesClient, error) { + backupClient := sql3.NewBackupShortTermRetentionPoliciesClientWithBaseURI(config.BaseURI(), creds.SubscriptionID()) + a, err := iam.GetResourceManagementAuthorizer(creds) + if err != nil { + return sql3.BackupShortTermRetentionPoliciesClient{}, err + } + backupClient.Authorizer = a + backupClient.AddToUserAgent(config.UserAgent()) + return backupClient, nil } diff --git a/pkg/resourcemanager/azuresql/azuresqlshared/sqlproperties.go b/pkg/resourcemanager/azuresql/azuresqlshared/sqlproperties.go index 2db95ca1412..814682d9424 100644 --- a/pkg/resourcemanager/azuresql/azuresqlshared/sqlproperties.go +++ b/pkg/resourcemanager/azuresql/azuresqlshared/sqlproperties.go @@ -83,6 +83,13 @@ type SQLFailoverGroupProperties struct { DatabaseList []string } +type SQLDatabaseBackupLongTermRetentionPolicy struct { + WeeklyRetention string + MonthlyRetention string + YearlyRetention string + WeekOfYear int32 +} + // SQLServerPropertiesToServer translates SQLServerProperties to ServerProperties func SQLServerPropertiesToServer(properties SQLServerProperties) (result sql.ServerProperties) { diff --git a/pkg/resourcemanager/azuresql/azuresqluser/azuresqluser_reconcile.go b/pkg/resourcemanager/azuresql/azuresqluser/azuresqluser_reconcile.go index fa43f09f8e0..e8fe0596103 100644 --- a/pkg/resourcemanager/azuresql/azuresqluser/azuresqluser_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqluser/azuresqluser_reconcile.go @@ -13,12 +13,13 @@ import ( "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" "github.com/Azure/azure-service-operator/pkg/secrets" + "github.com/google/uuid" + "k8s.io/apimachinery/pkg/runtime" + "github.com/Azure/azure-service-operator/api/v1alpha1" "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/resourcemanager" keyvaultSecrets "github.com/Azure/azure-service-operator/pkg/secrets/keyvault" - "github.com/google/uuid" - "k8s.io/apimachinery/pkg/runtime" _ "github.com/denisenkom/go-mssqldb" "k8s.io/apimachinery/pkg/types"