Feature/add public access in postgresql

[G5Olivieri]

Description of your changes

Add support to PublicNetworkAccess field in PostgreSQLServer.

Implement crossplane-contrib#240

I have:

• Read and followed Crossplane's contribution process.
• Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

• Unit tests
• Manually tested

Disclaimer

Remove minimalTlsVersion field or change sslEnforcement to Enabled for the controller not to enter an update loop, because that API ignore field minimalTlsVersion and always returns the value TLSEnforcementDisabled, causing what IsUpToDate returns false.

[Feggah]

Looks good in general, besides what I pointed out in the other comments, I'm wondering what caused the line //go:build !ignore_autogenerated to be added in all generated files, seems like duplicated. Do you know where it came from?

[G5Olivieri]

Looks good in general, besides what I pointed out in the other comments, I'm wondering what caused the line //go:build !ignore_autogenerated to be added in all generated files, seems like duplicated. Do you know where it came from?

Both the editor and make generate generate this line in these files

[Feggah]

The code LGTM, but when I tried to run the controller to test the behavior, it was always updating the resource with sslEnforcement: Disabled and it was at least updating the publicNetworkAccess accordingly.

Then I tried with sslEnforcement: Enabled and the Reconciler was trying to update the resource, but it was always getting "context deadline exceeded" and not updating the publicNetworkAccess field. TBH IDK if this behavior is a bug that was already present in the resource or not, but it is affecting our needs.

kubectl describe output:

Events:
  Type     Reason                         Age                    From                                                   Message
  ----     ------                         ----                   ----                                                   -------
  Normal   CreatedExternalResource        4m24s                  managed/postgresqlserver.database.azure.crossplane.io  Successfully requested creation of external resource
  Warning  CannotCreateExternalResource   4m20s                  managed/postgresqlserver.database.azure.crossplane.io  cannot create PostgreSQLServer: postgresql.ServersClient#Create: Failure sending request: StatusCode=0 -- Original Error: autorest/azure: Service returned an error. Status=<nil> Code="ServiceBusy" Message="Service is temporarily busy and the operation cannot be performed. Please try again later."
  Normal   UpdatedExternalResource        4m17s (x2 over 4m19s)  managed/postgresqlserver.database.azure.crossplane.io  Successfully requested update of external resource
  Warning  CannotObserveExternalResource  2m17s                  managed/postgresqlserver.database.azure.crossplane.io  cannot update PostgreSQL custom resource: Operation cannot be fulfilled on postgresqlservers.database.azure.crossplane.io "example-psql": the object has been modified; please apply your changes to the latest version and try again
  Warning  CannotUpdateExternalResource   17s (x3 over 2m17s)    managed/postgresqlserver.database.azure.crossplane.io  cannot update PostgreSQLServer: postgresql.ServersClient#Update: Failure sending request: StatusCode=0 -- Original Error: context deadline exceeded

That was my test:

YAML example to apply:

---
apiVersion: database.azure.crossplane.io/v1beta1
kind: PostgreSQLServer
metadata:
  name: example-psql
  labels:
    example: "true"
spec:
  forProvider:
    administratorLogin: myadmin
    resourceGroupName: gabriel-lab
    location: East US
    minimalTlsVersion: TLS12
    publicNetworkAccess: Disabled
    sslEnforcement: Enabled
    version: "9.6"
    sku:
      # Note that Basic servers do not support virtual network rules
      tier: GeneralPurpose
      capacity: 2
      family: Gen5
    storageProfile:
      storageMB: 20480
  writeConnectionSecretToRef:
    namespace: crossplane-system
    name: example-psql
  providerConfigRef:
    name: azure-provider-config

With this YAML, I got the Postgres server created:

Then I changed the publicNetworkAccess value to Enabled

...
    publicNetworkAccess: Enabled
...

And the resource in Azure was not updated.

[G5Olivieri]

The code LGTM, but when I tried to run the controller to test the behavior, it was always updating the resource with sslEnforcement: Disabled and it was at least updating the publicNetworkAccess accordingly.

Then I tried with sslEnforcement: Enabled and the Reconciler was trying to update the resource, but it was always getting "context deadline exceeded" and not updating the publicNetworkAccess field. TBH IDK if this behavior is a bug that was already present in the resource or not, but it is affecting our needs.

kubectl describe output:

Events:
  Type     Reason                         Age                    From                                                   Message
  ----     ------                         ----                   ----                                                   -------
  Normal   CreatedExternalResource        4m24s                  managed/postgresqlserver.database.azure.crossplane.io  Successfully requested creation of external resource
  Warning  CannotCreateExternalResource   4m20s                  managed/postgresqlserver.database.azure.crossplane.io  cannot create PostgreSQLServer: postgresql.ServersClient#Create: Failure sending request: StatusCode=0 -- Original Error: autorest/azure: Service returned an error. Status=<nil> Code="ServiceBusy" Message="Service is temporarily busy and the operation cannot be performed. Please try again later."
  Normal   UpdatedExternalResource        4m17s (x2 over 4m19s)  managed/postgresqlserver.database.azure.crossplane.io  Successfully requested update of external resource
  Warning  CannotObserveExternalResource  2m17s                  managed/postgresqlserver.database.azure.crossplane.io  cannot update PostgreSQL custom resource: Operation cannot be fulfilled on postgresqlservers.database.azure.crossplane.io "example-psql": the object has been modified; please apply your changes to the latest version and try again
  Warning  CannotUpdateExternalResource   17s (x3 over 2m17s)    managed/postgresqlserver.database.azure.crossplane.io  cannot update PostgreSQLServer: postgresql.ServersClient#Update: Failure sending request: StatusCode=0 -- Original Error: context deadline exceeded

That was my test:

YAML example to apply:

---
apiVersion: database.azure.crossplane.io/v1beta1
kind: PostgreSQLServer
metadata:
  name: example-psql
  labels:
    example: "true"
spec:
  forProvider:
    administratorLogin: myadmin
    resourceGroupName: gabriel-lab
    location: East US
    minimalTlsVersion: TLS12
    publicNetworkAccess: Disabled
    sslEnforcement: Enabled
    version: "9.6"
    sku:
      # Note that Basic servers do not support virtual network rules
      tier: GeneralPurpose
      capacity: 2
      family: Gen5
    storageProfile:
      storageMB: 20480
  writeConnectionSecretToRef:
    namespace: crossplane-system
    name: example-psql
  providerConfigRef:
    name: azure-provider-config

With this YAML, I got the Postgres server created:

Then I changed the publicNetworkAccess value to Enabled

...
    publicNetworkAccess: Enabled
...

And the resource in Azure was not updated.

This bug is prior to this PR.

The first note is that the value of the minimalTlsVersion is wrong in the example. The possible values to these field are TLS1_0, TLS1_1, TLS1_2 and TLSEnforcementDisabled.

The bug description

Scenario 1

When the sslEnforcement field is set to Enabled and the minimalTlsVersion is set with TLS12 value, the Azure API ignores the minimalTlsVersion field and set the value to TLSenforcementDisabled, because TLS12 is not valid value. In all cycle of the Reconcile, it check if field returned from the API is equal to Kubernetes object and always return false, falling the Update flow. So the Update of the minimalTlsVersion field is ignored and it generates an infinite update loop.

Scenario 2

When the sslEnforcement field is set to Disabled the minimalTlsVersion is always set to TLSEnforcementDisabled in Azure API and it generates an infinite update loop.

When is in infinite update loop is not possible update crossplane object. Only to delete.

Suggestion

Always use the pair

sslEnforcement: Enabled
minimalTlsVersion: TLS1_2

or

sslEnforcement: Disabled
minimalTlsVersion: TLSenforcementDisabled

[Feggah]

Thanks for the explanation @G5Olivieri . I've runned the controller with the values that you provided and it is working as expected now. I'm kinda surprised that this behavior is in the main crossplane repo, we could discuss later if we fix it in other PR. Good job 🚀

Just a quick tip, when you open this PR to Crossplane, I think you should mark @hasheddan in the PR comments (because this was a TODO with his name) and then send the PR link in Crossplane Slack.