Merge pull request #1835 from wotolom/rdsinstance-CloudwatchLogsExpor…

…tConfiguration-fix

apis/database/v1beta1/rdsinstance_types.go

@@ -57,24 +57,6 @@ type ProcessorFeature struct {
 	Value string `json:"value"`
 }
 
-// CloudwatchLogsExportConfiguration is the configuration setting for the log types to be enabled for export to CloudWatch
-// Logs for a specific DB instance or DB cluster.
-// The EnableLogTypes and DisableLogTypes arrays determine which logs will be
-// exported (or not exported) to CloudWatch Logs. The values within these arrays
-// depend on the DB engine being used. For more information, see Publishing
-// Database Logs to Amazon CloudWatch Logs  (http://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_LogAccess.html#USER_LogAccess.Procedural.UploadtoCloudWatch)
-// in the Amazon RDS User Guide.
-// Please also see https://docs.aws.amazon.com/goto/WebAPI/rds-2014-10-31/CloudwatchLogsExportConfiguration
-type CloudwatchLogsExportConfiguration struct {
-	// DisableLogTypes is the list of log types to disable.
-	// +immutable
-	DisableLogTypes []string `json:"disableLogTypes,omitempty"`
-
-	// EnableLogTypes is the list of log types to enable.
-	// +immutable
-	EnableLogTypes []string `json:"enableLogTypes,omitempty"`
-}
-
 // ScalingConfiguration contains the scaling configuration of an Aurora Serverless DB cluster.
 // For more information, see Using Amazon Aurora Serverless (http://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/aurora-serverless.html)
 // in the Amazon Aurora User Guide.
@@ -413,7 +395,6 @@ type RDSInstanceParameters struct {
 	// Logs. The values in the list depend on the DB engine being used. For more
 	// information, see Publishing Database Logs to Amazon CloudWatch Logs  (http://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_LogAccess.html#USER_LogAccess.Procedural.UploadtoCloudWatch)
 	// in the Amazon Relational Database Service User Guide.
-	// +immutable
 	// +optional
 	EnableCloudwatchLogsExports []string `json:"enableCloudwatchLogsExports,omitempty"`
 
@@ -793,12 +774,6 @@ type RDSInstanceParameters struct {
 	// +optional
 	ApplyModificationsImmediately *bool `json:"applyModificationsImmediately,omitempty"`
 
-	// CloudwatchLogsExportConfiguration is the configuration setting for the log types to be enabled for export to CloudWatch
-	// Logs for a specific DB instance.
-	// +immutable
-	// +optional
-	CloudwatchLogsExportConfiguration *CloudwatchLogsExportConfiguration `json:"cloudwatchLogsExportConfiguration,omitempty"`
-
 	// DBParameterGroupName is the name of the DB parameter group to associate with this DB instance. If
 	// this argument is omitted, the default DBParameterGroup for the specified
 	// engine is used.
@@ -1166,6 +1141,13 @@ type RDSInstanceObservation struct {
 	// InstanceCreateTime provides the date and time the DB instance was created.
 	InstanceCreateTime *metav1.Time `json:"instanceCreateTime,omitempty"`
 
+	// A list of log types that this DB instance is configured to export to CloudWatch
+	// Logs. Log types vary by DB engine. For information about the log types for each
+	// DB engine, see Amazon RDS Database Log Files
+	// (https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_LogAccess.html) in
+	// the Amazon RDS User Guide.
+	EnabledCloudwatchLogsExports []string `json:"enabledCloudwatchLogsExports,omitempty"`
+
 	// Endpoint specifies the connection endpoint.
 	Endpoint Endpoint `json:"endpoint,omitempty"`
 

package/crds/database.aws.crossplane.io_rdsinstances.yaml

@@ -170,22 +170,6 @@ spec:
                       engines, Amazon Aurora Not applicable. The character set is
                       managed by the DB cluster. For more information, see CreateDBCluster.
                     type: string
-                  cloudwatchLogsExportConfiguration:
-                    description: CloudwatchLogsExportConfiguration is the configuration
-                      setting for the log types to be enabled for export to CloudWatch
-                      Logs for a specific DB instance.
-                    properties:
-                      disableLogTypes:
-                        description: DisableLogTypes is the list of log types to disable.
-                        items:
-                          type: string
-                        type: array
-                      enableLogTypes:
-                        description: EnableLogTypes is the list of log types to enable.
-                        items:
-                          type: string
-                        type: array
-                    type: object
                   copyTagsToSnapshot:
                     description: CopyTagsToSnapshot should be true to copy all tags
                       from the DB instance to snapshots of the DB instance, and otherwise
@@ -1546,6 +1530,15 @@ spec:
                           type: string
                       type: object
                     type: array
+                  enabledCloudwatchLogsExports:
+                    description: A list of log types that this DB instance is configured
+                      to export to CloudWatch Logs. Log types vary by DB engine. For
+                      information about the log types for each DB engine, see Amazon
+                      RDS Database Log Files (https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_LogAccess.html)
+                      in the Amazon RDS User Guide.
+                    items:
+                      type: string
+                    type: array
                   endpoint:
                     description: Endpoint specifies the connection endpoint.
                     properties:

pkg/clients/database/rds.go

@@ -359,7 +359,7 @@ func CreatePatch(in *rdstypes.DBInstance, target *v1beta1.RDSInstanceParameters)
 }
 
 // GenerateModifyDBInstanceInput from RDSInstanceSpec
-func GenerateModifyDBInstanceInput(name string, p *v1beta1.RDSInstanceParameters) *rds.ModifyDBInstanceInput {
+func GenerateModifyDBInstanceInput(name string, p *v1beta1.RDSInstanceParameters, db *rdstypes.DBInstance) *rds.ModifyDBInstanceInput {
 	// NOTE(muvaf): MasterUserPassword is not used here. So, password is set once
 	// and kept that way.
 	// NOTE(muvaf): Change of DBInstanceIdentifier is supported by AWS but
@@ -411,12 +411,11 @@ func GenerateModifyDBInstanceInput(name string, p *v1beta1.RDSInstanceParameters
 			}
 		}
 	}
-	if p.CloudwatchLogsExportConfiguration != nil {
-		m.CloudwatchLogsExportConfiguration = &rdstypes.CloudwatchLogsExportConfiguration{
-			DisableLogTypes: p.CloudwatchLogsExportConfiguration.DisableLogTypes,
-			EnableLogTypes:  p.CloudwatchLogsExportConfiguration.EnableLogTypes,
-		}
-	}
+
+	m.CloudwatchLogsExportConfiguration = generateCloudWatchExportConfiguration(
+		p.EnableCloudwatchLogsExports,
+		db.EnabledCloudwatchLogsExports)
+
 	return m
 }
 
@@ -431,6 +430,7 @@ func GenerateObservation(db rdstypes.DBInstance) v1beta1.RDSInstanceObservation
 		DBInstanceArn:                         aws.ToString(db.DBInstanceArn),
 		DBInstancePort:                        int(db.DbInstancePort),
 		DBResourceID:                          aws.ToString(db.DbiResourceId),
+		EnabledCloudwatchLogsExports:          db.EnabledCloudwatchLogsExports,
 		EnhancedMonitoringResourceArn:         aws.ToString(db.EnhancedMonitoringResourceArn),
 		PerformanceInsightsEnabled:            aws.ToBool(db.PerformanceInsightsEnabled),
 		ReadReplicaDBClusterIdentifiers:       db.ReadReplicaDBClusterIdentifiers,
@@ -621,9 +621,6 @@ func LateInitialize(in *v1beta1.RDSInstanceParameters, db *rdstypes.DBInstance)
 	if aws.ToString(in.DBSubnetGroupName) == "" && db.DBSubnetGroup != nil {
 		in.DBSubnetGroupName = db.DBSubnetGroup.DBSubnetGroupName
 	}
-	if len(in.EnableCloudwatchLogsExports) == 0 && len(db.EnabledCloudwatchLogsExports) != 0 {
-		in.EnableCloudwatchLogsExports = db.EnabledCloudwatchLogsExports
-	}
 	if len(in.ProcessorFeatures) == 0 && len(db.ProcessorFeatures) != 0 {
 		in.ProcessorFeatures = make([]v1beta1.ProcessorFeature, len(db.ProcessorFeatures))
 		for i, val := range db.ProcessorFeatures {
@@ -678,13 +675,21 @@ func IsUpToDate(ctx context.Context, kube client.Client, r *v1beta1.RDSInstance,
 		cmpopts.IgnoreFields(v1beta1.RDSInstanceParameters{}, "AllowMajorVersionUpgrade"),
 		cmpopts.IgnoreFields(v1beta1.RDSInstanceParameters{}, "MasterPasswordSecretRef"),
 		cmpopts.IgnoreFields(v1beta1.RDSInstanceParameters{}, "OptionGroupName"),
+		cmpopts.IgnoreFields(v1beta1.RDSInstanceParameters{}, "EnableCloudwatchLogsExports"),
 	)
 
 	engineVersionChanged := !isEngineVersionUpToDate(r, db)
 
 	optionGroupChanged := !isOptionGroupUpToDate(r, db)
 
-	if diff == "" && !pwdChanged && !engineVersionChanged && !optionGroupChanged {
+	cloudwatchLogsExportChanged := false
+	// only check CloudwatchLogsExports if there are no pending cloudwatchlogs exports (ignores the apply immediately setting)
+	// (to avoid: "api error InvalidParameterCombination: You cannot configure CloudWatch Logs while a previous configuration is in progress.")
+	if db.PendingModifiedValues != nil && db.PendingModifiedValues.PendingCloudwatchLogsExports == nil {
+		cloudwatchLogsExportChanged = !areSameElements(r.Spec.ForProvider.EnableCloudwatchLogsExports, db.EnabledCloudwatchLogsExports)
+	}
+
+	if diff == "" && !pwdChanged && !engineVersionChanged && !optionGroupChanged && !cloudwatchLogsExportChanged {
 		return true, "", nil
 	}
 
@@ -780,3 +785,52 @@ func GetConnectionDetails(in v1beta1.RDSInstance) managed.ConnectionDetails {
 		xpv1.ResourceCredentialsSecretPortKey:     []byte(strconv.Itoa(in.Status.AtProvider.Endpoint.Port)),
 	}
 }
+
+func generateCloudWatchExportConfiguration(spec, current []string) *rdstypes.CloudwatchLogsExportConfiguration {
+	toEnable := []string{}
+	toDisable := []string{}
+
+	currentMap := make(map[string]struct{}, len(current))
+	for _, currentID := range current {
+		currentMap[currentID] = struct{}{}
+	}
+
+	specMap := make(map[string]struct{}, len(spec))
+	for _, specID := range spec {
+		specMap[specID] = struct{}{}
+
+		if _, exists := currentMap[specID]; !exists {
+			toEnable = append(toEnable, specID)
+		}
+	}
+
+	for _, currentID := range current {
+		if _, exists := specMap[currentID]; !exists {
+			toDisable = append(toDisable, currentID)
+		}
+	}
+
+	return &rdstypes.CloudwatchLogsExportConfiguration{
+		EnableLogTypes:  toEnable,
+		DisableLogTypes: toDisable,
+	}
+}
+
+func areSameElements(a1, a2 []string) bool {
+	if len(a1) != len(a2) {
+		return false
+	}
+
+	m2 := make(map[string]struct{}, len(a2))
+	for _, s2 := range a2 {
+		m2[s2] = struct{}{}
+	}
+
+	for _, s1 := range a1 {
+		if _, exists := m2[s1]; !exists {
+			return false
+		}
+	}
+
+	return true
+}