-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
External dbs observability and management v2 #856
base: main
Are you sure you want to change the base?
Conversation
} | ||
} | ||
|
||
variable "external_cdb_connector_connection_credentials_password" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about adding a validation? Like value != ""?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Have provided !="" validation for credential and connection variables to make sure it´s set to something before applying anything (See commit: "Connection and credential variables validated for empty strings"
} | ||
} | ||
|
||
variable "external_pdb_connector_connection_credentials_password" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about adding a validation, like value != ""?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Have provided !="" validation for credential and connection variables to make sure it´s set to something before applying anything (See commit: "Connection and credential variables validated for empty strings")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a warning that the password values may be present in clear text in the tfstate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. See commit: 1203619
At line 53, it´s noted that the Terraform config will set port to 1521 if no value is provided in JSON-input At line 84-85, it´s noted that credential details are visible as plain text in both the JSON-input and in the tfstate, so both files should be managed securely
In variables for DB system connections, it´s validated whether usernames, passwords, hosts, service names, and agent IDs are provided in JSON-input as empty strings. This is to stop apply early before anything is actually created in OCI Port and protocol variable definitions have not been changed since the former has a default value and the latter has alternative validation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes.
condition = var.external_database_connector_agent_id != "" | ||
error_message = "The value of 'managementAgentId' in the JSON-input for database systems is an empty string" | ||
} | ||
#default = "ocid1.managementagent.oc1..XXXXXXX" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Olaf, are you referring to removing the default value parameters in these files you are mentioning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifically the ones where there are placeholder OCIDs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed placeholder OCIDs in the three variable.tf-files specified. See commit c8b0810
variable "external_database_connector_agent_id" { | ||
description = "The OCID for the management agent used for database connections" | ||
type = string | ||
#default = "ocid1.managementagent.oc1..XXXXXXX" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed placeholder OCIDs in the three variable.tf-files specified. See commit c8b0810
variable "compartment_ocid" { | ||
description = "Compartment OCID" | ||
type = string | ||
default = "ocid1.compartment.oc1..XXXXXX" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed placeholder OCIDs in the three variable.tf-files specified. See commit c8b0810
Default OCID variable value placeholders removed from variable files in the root, container_connection, and pdb module
NOTE: New pull request after adding password placeholders in db_credentials.json and db_credentials_example.json
I made a Terraform configuration to enable the following services in Observability & Management for external databases with agents.
The configuration will do the setup based on input from two JSON-files: db_systems.json and db_credentials.json