Merge branch 'AbhiK_tcps_bugfix5' into 'master'

Patching the svc while changing type from LB to NodePort, max cert validity 1yr

See merge request rac-docker-dev/oracle-database-operator!229

Author: yunus-qureshi

apis/database/v1alpha1/singleinstancedatabase_webhook.go

@@ -270,12 +270,12 @@ func (r *SingleInstanceDatabase) ValidateCreate() error {
 				field.Invalid(field.NewPath("spec").Child("tcpsCertRenewInterval"), r.Spec.TcpsCertRenewInterval,
 					"Please provide valid string to parse the tcpsCertRenewInterval."))
 		}
-		maxLimit, _ := time.ParseDuration("26280h")
+		maxLimit, _ := time.ParseDuration("8760h")
 		minLimit, _ := time.ParseDuration("24h")
 		if duration > maxLimit || duration < minLimit {
 			allErrs = append(allErrs,
 				field.Invalid(field.NewPath("spec").Child("tcpsCertRenewInterval"), r.Spec.TcpsCertRenewInterval,
-					"Please specify tcpsCertRenewInterval in the range: 24h to 26280h"))
+					"Please specify tcpsCertRenewInterval in the range: 24h to 8760h"))
 		}
 	}
 	if len(allErrs) == 0 {

commons/database/constants.go

@@ -501,10 +501,10 @@ const ClientWalletLocation string = "/opt/oracle/oradata/clientWallet/%s"
 
 // Service Patch Payloads
 // Three port payload: one OEM express, one TCP and one TCPS port
-const ThreePortPayload string = "{\"spec\": { \"ports\": [{\"name\": \"xmldb\", \"port\": 5500, \"protocol\": \"TCP\"},{%s},{%s}]}}"
+const ThreePortPayload string = "{\"spec\": { \"type\": \"%s\", \"ports\": [{\"name\": \"xmldb\", \"port\": 5500, \"protocol\": \"TCP\"},{%s},{%s}]}}"
 
 // Two port payload: one OEM express, one TCP/TCPS port
-const TwoPortPayload string = "{\"spec\": { \"ports\": [{\"name\": \"xmldb\", \"port\": 5500, \"protocol\": \"TCP\"},{%s}]}}"
+const TwoPortPayload string = "{\"spec\": { \"type\": \"%s\", \"ports\": [{\"name\": \"xmldb\", \"port\": 5500, \"protocol\": \"TCP\"},{%s}]}}"
 
 // Payload section for listener port
 const LsnrPort string = "\"name\": \"listener\", \"protocol\": \"TCP\", \"port\": %d, \"targetPort\": 1521"

config/crd/bases/database.oracle.com_cdbs.yaml

@@ -65,8 +65,6 @@ spec:
           spec:
             description: CDBSpec defines the desired state of CDB
             properties:
-              TestVariable:
-                type: string
               cdbAdminPwd:
                 description: Password for the CDB Administrator to manage PDB lifecycle
                 properties:

config/samples/sidb/singleinstancedatabase.yaml

@@ -48,10 +48,10 @@ spec:
   enableTCPS: false
 
   ## TCPS Certificate Renewal Interval: The time after which TCPS certificate will be renewed if TCPS connections are enabled.
-  ## tcpsCertRenewInterval can be in hours(h), minutes(m) and seconds(s); e.g. 17520h, 8760h etc.
-  ## Maximum value is 26280h (3 years), Minimum value is 24h; Default value is 17520h (2 years)
+  ## tcpsCertRenewInterval can be in hours(h), minutes(m) and seconds(s); e.g. 4380h, 8760h etc.
+  ## Maximum value is 8760h (1 year), Minimum value is 24h; Default value is 8760h (1 year)
   ## If this field is commented out/removed from the yaml, it will disable the auto-renewal feature for TCPS certificate
-  tcpsCertRenewInterval: 17520h
+  tcpsCertRenewInterval: 8760h
 
   ## NA if cloning from a SourceDB (cloneFrom is set)
   ## Specify both sgaSize and pgaSize (in MB) or dont specify both

config/samples/sidb/singleinstancedatabase_tcps.yaml

@@ -35,10 +35,10 @@ spec:
   enableTCPS: true
 
   ## TCPS Certificate Renewal Interval: The time after which TCPS certificate will be renewed if TCPS connections are enabled.
-  ## tcpsCertRenewInterval can be in hours(h), minutes(m) and seconds(s); e.g. 17520h, 8760h etc.
-  ## Maximum value is 26280h (3 years), Minimum value is 24h; Default value is 17520h (2 years)
+  ## tcpsCertRenewInterval can be in hours(h), minutes(m) and seconds(s); e.g. 4380h, 8760h etc.
+  ## Maximum value is 8760h (1 year), Minimum value is 24h; Default value is 8760h (1 year)
   ## If this field is commented out/removed from the yaml, it will disable the auto-renewal feature for TCPS certificate
-  tcpsCertRenewInterval: 17520h
+  tcpsCertRenewInterval: 8760h
 
   ## Database image details
   image:

controllers/database/singleinstancedatabase_controller.go

@@ -1126,57 +1126,37 @@ func (r *SingleInstanceDatabaseReconciler) createOrReplaceSVC(ctx context.Contex
 			}
 		}
 
-		deleteSvc := false
 		patchSvc := false
-		if extSvc.Spec.Type != extSvcType {
-			deleteSvc = true
-		} else {
-			// Conditions to determine whether to patch or not
-			if len(extSvc.Spec.Ports) != requiredPorts {
-				patchSvc = true
-			}
 
-			if (m.Spec.ListenerPort != 0 && svcPort != targetPorts[1]) || (m.Spec.EnableTCPS && m.Spec.TcpsListenerPort != 0 && tcpsSvcPort != targetPorts[len(targetPorts)-1]) {
-				patchSvc = true
-			}
+		// Conditions to determine whether to patch or not
+		if extSvc.Spec.Type != extSvcType || len(extSvc.Spec.Ports) != requiredPorts {
+			patchSvc = true
+		}
 
-			if m.Spec.LoadBalancer {
-				if m.Spec.EnableTCPS {
-					if m.Spec.TcpsListenerPort == 0 && tcpsSvcPort != targetPorts[len(targetPorts)-1] {
-						patchSvc = true
-					}
-				} else {
-					if m.Spec.ListenerPort == 0 && svcPort != targetPorts[1] {
-						patchSvc = true
-					}
+		if (m.Spec.ListenerPort != 0 && svcPort != targetPorts[1]) || (m.Spec.EnableTCPS && m.Spec.TcpsListenerPort != 0 && tcpsSvcPort != targetPorts[len(targetPorts)-1]) {
+			patchSvc = true
+		}
+
+		if m.Spec.LoadBalancer {
+			if m.Spec.EnableTCPS {
+				if m.Spec.TcpsListenerPort == 0 && tcpsSvcPort != targetPorts[len(targetPorts)-1] {
+					patchSvc = true
 				}
 			} else {
-				if m.Spec.EnableTCPS {
-					if m.Spec.TcpsListenerPort == 0 && tcpsSvcPort != extSvc.Spec.Ports[len(targetPorts)-1].TargetPort.IntVal {
-						patchSvc = true
-					}
-				} else {
-					if m.Spec.ListenerPort == 0 && svcPort != extSvc.Spec.Ports[1].TargetPort.IntVal {
-						patchSvc = true
-					}
+				if m.Spec.ListenerPort == 0 && svcPort != targetPorts[1] {
+					patchSvc = true
 				}
 			}
-		}
-
-		if deleteSvc {
-			// Deleting th service
-			log.Info("Deleting service", "name", extSvcName)
-			// Setting GracePeriodSeconds to 0 for instant deletion
-			delOpts := &client.DeleteOptions{}
-			var gracePeriod client.GracePeriodSeconds = 0
-			gracePeriod.ApplyToDelete(delOpts)
-
-			err := r.Delete(ctx, extSvc, delOpts)
-			if err != nil {
-				r.Log.Error(err, "Failed to delete service", "name", extSvcName)
-				return requeueN, err
+		} else {
+			if m.Spec.EnableTCPS {
+				if m.Spec.TcpsListenerPort == 0 && tcpsSvcPort != extSvc.Spec.Ports[len(targetPorts)-1].TargetPort.IntVal {
+					patchSvc = true
+				}
+			} else {
+				if m.Spec.ListenerPort == 0 && svcPort != extSvc.Spec.Ports[1].TargetPort.IntVal {
+					patchSvc = true
+				}
 			}
-			isExtSvcFound = false
 		}
 
 		if patchSvc {
@@ -1193,29 +1173,29 @@ func (r *SingleInstanceDatabaseReconciler) createOrReplaceSVC(ctx context.Contex
 			if m.Spec.LoadBalancer {
 				if m.Spec.EnableTCPS {
 					if m.Spec.ListenerPort != 0 {
-						payload = fmt.Sprintf(dbcommons.ThreePortPayload, fmt.Sprintf(dbcommons.LsnrPort, svcPort), fmt.Sprintf(dbcommons.TcpsPort, tcpsSvcPort))
+						payload = fmt.Sprintf(dbcommons.ThreePortPayload, extSvcType, fmt.Sprintf(dbcommons.LsnrPort, svcPort), fmt.Sprintf(dbcommons.TcpsPort, tcpsSvcPort))
 					} else {
-						payload = fmt.Sprintf(dbcommons.TwoPortPayload, fmt.Sprintf(dbcommons.TcpsPort, tcpsSvcPort))
+						payload = fmt.Sprintf(dbcommons.TwoPortPayload, extSvcType, fmt.Sprintf(dbcommons.TcpsPort, tcpsSvcPort))
 					}
 				} else {
-					payload = fmt.Sprintf(dbcommons.TwoPortPayload, fmt.Sprintf(dbcommons.LsnrPort, svcPort))
+					payload = fmt.Sprintf(dbcommons.TwoPortPayload, extSvcType, fmt.Sprintf(dbcommons.LsnrPort, svcPort))
 				}
 			} else {
 				if m.Spec.EnableTCPS {
 					if m.Spec.ListenerPort != 0 && m.Spec.TcpsListenerPort != 0 {
-						payload = fmt.Sprintf(dbcommons.ThreePortPayload, fmt.Sprintf(dbcommons.LsnrNodePort, svcPort), fmt.Sprintf(dbcommons.TcpsNodePort, tcpsSvcPort))
+						payload = fmt.Sprintf(dbcommons.ThreePortPayload, extSvcType, fmt.Sprintf(dbcommons.LsnrNodePort, svcPort), fmt.Sprintf(dbcommons.TcpsNodePort, tcpsSvcPort))
 					} else if m.Spec.ListenerPort != 0 {
-						payload = fmt.Sprintf(dbcommons.ThreePortPayload, fmt.Sprintf(dbcommons.LsnrNodePort, svcPort), fmt.Sprintf(dbcommons.TcpsPort, tcpsSvcPort))
+						payload = fmt.Sprintf(dbcommons.ThreePortPayload, extSvcType, fmt.Sprintf(dbcommons.LsnrNodePort, svcPort), fmt.Sprintf(dbcommons.TcpsPort, tcpsSvcPort))
 					} else if m.Spec.TcpsListenerPort != 0 {
-						payload = fmt.Sprintf(dbcommons.TwoPortPayload, fmt.Sprintf(dbcommons.TcpsNodePort, tcpsSvcPort))
+						payload = fmt.Sprintf(dbcommons.TwoPortPayload, extSvcType, fmt.Sprintf(dbcommons.TcpsNodePort, tcpsSvcPort))
 					} else {
-						payload = fmt.Sprintf(dbcommons.TwoPortPayload, fmt.Sprintf(dbcommons.TcpsPort, tcpsSvcPort))
+						payload = fmt.Sprintf(dbcommons.TwoPortPayload, extSvcType, fmt.Sprintf(dbcommons.TcpsPort, tcpsSvcPort))
 					}
 				} else {
 					if m.Spec.ListenerPort != 0 {
-						payload = fmt.Sprintf(dbcommons.TwoPortPayload, fmt.Sprintf(dbcommons.LsnrNodePort, svcPort))
+						payload = fmt.Sprintf(dbcommons.TwoPortPayload, extSvcType, fmt.Sprintf(dbcommons.LsnrNodePort, svcPort))
 					} else {
-						payload = fmt.Sprintf(dbcommons.TwoPortPayload, fmt.Sprintf(dbcommons.LsnrPort, svcPort))
+						payload = fmt.Sprintf(dbcommons.TwoPortPayload, extSvcType, fmt.Sprintf(dbcommons.LsnrPort, svcPort))
 					}
 				}
 			}

docs/sidb/README.md

@@ -543,8 +543,8 @@ The following steps are required to connect the Database using TCPS:
 **NOTE:**
 - Only database server authentication is supported (no mTLS).
 - When TCPS is enabled, a self-signed certificate is generated and stored inside the wallets. For users' convenience, a client-side wallet is generated and stored at `/opt/oracle/oradata/clientWallet/$ORACLE_SID` location in the pod.
-- The self-signed certificate used with TCPS has validity for 2 years. After the certificate is expired, it will be renewed by the `OraOperator` automatically. You need to download the wallet again after the auto-renewal.
-- You can set the certificate renew interval with the help of `tcpsCertRenewInterval` field in the **[config/samples/sidb/singleinstancedatabase.yaml](../../config/samples/sidb/singleinstancedatabase.yaml)** file. The minimum accepted value is 24h, and the maximum value is 26280h (3 years). The certificates used with TCPS will automatically be renewed after this interval. If this field is omitted/commented in the yaml file, the certificates will not be renewed automatically.
+- The self-signed certificate used with TCPS has validity for 1 year. After the certificate is expired, it will be renewed by the `OraOperator` automatically. You need to download the wallet again after the auto-renewal.
+- You can set the certificate renew interval with the help of `tcpsCertRenewInterval` field in the **[config/samples/sidb/singleinstancedatabase.yaml](../../config/samples/sidb/singleinstancedatabase.yaml)** file. The minimum accepted value is 24h, and the maximum value is 8760h (1 year). The certificates used with TCPS will automatically be renewed after this interval. If this field is omitted/commented in the yaml file, the certificates will not be renewed automatically.
 - When the certificate gets created/renewed, the `.status.certCreationTimestamp` status variable gets updated accordingly. You can see this timestamp by using the following command:
   ```bash
   kubectl get singleinstancedatabase sidb-sample  -o "jsonpath={.status.certCreationTimestamp}"