Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions api/v1alpha1/trillian_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,24 @@ type TrillianDB struct {
//+kubebuilder:default:=true
//+kubebuilder:validation:XValidation:rule=(self == oldSelf),message=Field is immutable
Create *bool `json:"create"`
// Secret with values to be used to connect to an existing DB or to be used with the creation of a new DB
// DB provider. Supported are mysql and postgresql.
//+kubebuilder:validation:Enum={postgresql,mysql}
//+kubebuilder:default:=mysql
Provider string `json:"provider,omitempty"`
// Secret with values to be used to connect to an existing DB or to be used with the creation of a new MySQL DB
// For MySQL, the secret must contain the following keys:
// mysql-host: The host of the MySQL server
// mysql-port: The port of the MySQL server
// mysql-user: The user to connect to the MySQL server
// mysql-password: The password to connect to the MySQL server
// mysql-database: The database to connect to
//+optional
// For PostgreSQL, only connection is supported, the secret must contain:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned about the new set of variables. Could we consider using the data source URL, similar to how we handle the search index? https://github.com/securesign/secure-sign-operator/blob/main/api/v1alpha1/rekor_types.go#L154

Envs are mounted by https://github.com/securesign/secure-sign-operator/blob/main/api/v1alpha1/rekor_types.go#L55
Any changes must be implemented carefully to maintain backward compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we consider using the data source URL:

Do you mean for both providers (mysql and postgresql) ?

If it's only for postgresql, I have added env variables for postgresql to have a similar config as mysql to be able to re-use the existing code in server-deployment.go

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, for both. I would like to have unified API for all components. The searchIndex uses the following format (see the data source spec https://jdbc.postgresql.org/documentation/use/#connecting-to-the-database):
"$(MYSQL_USER):$(MYSQL_PASSWORD)@tcp(my-mysql.$(NAMESPACE).svc:3300)/$(MYSQL_DB)"
https://github.com/securesign/secure-sign-operator/blob/main/test/e2e/byodb_test.go#L43

I know, that this could mean an API change that we can't afford in a minor version release. I just want to bring this to the table and explore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about changing searchIndex config to be as trillian DB's ? I find it easier to be handled by the customers.
I think providing the full database URL puts too much responsibility on the customer and increases the chance of mistakes. Supplying only the essential variables allows the operator to assemble the URL correctly and consistently.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Utilizing a URL to pass the datasource is a widely accepted convention. This method provides the flexibility to embed numerous configuration parameters, such as those relating to TLS (Transport Layer Security). Conversely, the discovery of proprietary parameter names may present challenges and is susceptible to error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so if we apply it on the trillian mysql db, won't this break upgrade scenario and backward compatibility ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about providing a default URL that will read content from the secret. The secret needs to be mounted as Auth (https://github.com/securesign/secure-sign-operator/blob/main/internal/controller/rekor/actions/server/deployment.go#L82). Do you think it is doable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this would break the upgrade scenario anyway, right ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so.

  • The URL will have default value -> no braking change
  • the credentials file will be just mapped diferently to the deployment but it will be still working

// postgresql-host: The host of the PostgreSQL server
// postgresql-port: The port of the PostgreSQL server
// postgresql-user: The user to connect to the PostgreSQL server
// postgresql-password: The password to connect to the PostgreSQL server
// postgresql-database: The database to connect to
// +optional
DatabaseSecretRef *LocalObjectReference `json:"databaseSecretRef,omitempty"`
// PVC configuration
//+kubebuilder:default:={size: "5Gi", retain: true}
Expand Down
3 changes: 3 additions & 0 deletions api/v1alpha1/trillian_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
)

var _ = Describe("Trillian", func() {
const mysqlProvider = "mysql"

Context("TrillianSpec", func() {
It("can be created", func() {
Expand Down Expand Up @@ -201,6 +202,7 @@ var _ = Describe("Trillian", func() {
Namespace: "default",
},
}
expectedTrillianInstance.Spec.Db.Provider = mysqlProvider

Expect(k8sClient.Create(context.Background(), &trillianInstance)).To(Succeed())
fetched := &Trillian{}
Expand Down Expand Up @@ -262,6 +264,7 @@ var _ = Describe("Trillian", func() {
expectedTrillianInstance.Spec.Db.DatabaseSecretRef = &LocalObjectReference{
Name: "secret",
}
expectedTrillianInstance.Spec.Db.Provider = mysqlProvider

Expect(k8sClient.Create(context.Background(), &trillianInstance)).To(Succeed())
fetchedTrillian := &Trillian{}
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 15 additions & 1 deletion config/crd/bases/rhtas.redhat.com_securesigns.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5362,12 +5362,19 @@ spec:
rule: (self == oldSelf)
databaseSecretRef:
description: |-
Secret with values to be used to connect to an existing DB or to be used with the creation of a new DB
Secret with values to be used to connect to an existing DB or to be used with the creation of a new MySQL DB
For MySQL, the secret must contain the following keys:
mysql-host: The host of the MySQL server
mysql-port: The port of the MySQL server
mysql-user: The user to connect to the MySQL server
mysql-password: The password to connect to the MySQL server
mysql-database: The database to connect to
For PostgreSQL, only connection is supported, the secret must contain:
postgresql-host: The host of the PostgreSQL server
postgresql-port: The port of the PostgreSQL server
postgresql-user: The user to connect to the PostgreSQL server
postgresql-password: The password to connect to the PostgreSQL server
postgresql-database: The database to connect to
properties:
name:
description: |-
Expand All @@ -5378,6 +5385,13 @@ spec:
- name
type: object
x-kubernetes-map-type: atomic
provider:
default: mysql
description: DB provider. Supported are mysql and postgresql.
enum:
- postgresql
- mysql
type: string
pvc:
default:
retain: true
Expand Down
32 changes: 30 additions & 2 deletions config/crd/bases/rhtas.redhat.com_trillians.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,19 @@ spec:
rule: (self == oldSelf)
databaseSecretRef:
description: |-
Secret with values to be used to connect to an existing DB or to be used with the creation of a new DB
Secret with values to be used to connect to an existing DB or to be used with the creation of a new MySQL DB
For MySQL, the secret must contain the following keys:
mysql-host: The host of the MySQL server
mysql-port: The port of the MySQL server
mysql-user: The user to connect to the MySQL server
mysql-password: The password to connect to the MySQL server
mysql-database: The database to connect to
For PostgreSQL, only connection is supported, the secret must contain:
postgresql-host: The host of the PostgreSQL server
postgresql-port: The port of the PostgreSQL server
postgresql-user: The user to connect to the PostgreSQL server
postgresql-password: The password to connect to the PostgreSQL server
postgresql-database: The database to connect to
properties:
name:
description: |-
Expand All @@ -80,6 +87,13 @@ spec:
- name
type: object
x-kubernetes-map-type: atomic
provider:
default: mysql
description: DB provider. Supported are mysql and postgresql.
enum:
- postgresql
- mysql
type: string
pvc:
default:
retain: true
Expand Down Expand Up @@ -2441,12 +2455,19 @@ spec:
rule: (self == oldSelf)
databaseSecretRef:
description: |-
Secret with values to be used to connect to an existing DB or to be used with the creation of a new DB
Secret with values to be used to connect to an existing DB or to be used with the creation of a new MySQL DB
For MySQL, the secret must contain the following keys:
mysql-host: The host of the MySQL server
mysql-port: The port of the MySQL server
mysql-user: The user to connect to the MySQL server
mysql-password: The password to connect to the MySQL server
mysql-database: The database to connect to
For PostgreSQL, only connection is supported, the secret must contain:
postgresql-host: The host of the PostgreSQL server
postgresql-port: The port of the PostgreSQL server
postgresql-user: The user to connect to the PostgreSQL server
postgresql-password: The password to connect to the PostgreSQL server
postgresql-database: The database to connect to
properties:
name:
description: |-
Expand All @@ -2457,6 +2478,13 @@ spec:
- name
type: object
x-kubernetes-map-type: atomic
provider:
default: mysql
description: DB provider. Supported are mysql and postgresql.
enum:
- postgresql
- mysql
type: string
pvc:
default:
retain: true
Expand Down
4 changes: 2 additions & 2 deletions config/default/images.env
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
RELATED_IMAGE_TRILLIAN_LOG_SIGNER=registry.redhat.io/rhtas/trillian-logsigner-rhel9@sha256:358d52e57faf07d7876f53902496e6926c39a9ac1f52a3b7dc1fab0d9d6d97c0
RELATED_IMAGE_TRILLIAN_LOG_SERVER=registry.redhat.io/rhtas/trillian-logserver-rhel9@sha256:9ecb8cbb0e1a1d3043a377992ad1795f01ae7491b04ea8e9914263361fc9d51c
RELATED_IMAGE_TRILLIAN_LOG_SIGNER=registry.redhat.io/rhtas/trillian-logsigner-rhel9@sha256:7aee902e70f430f7502b99efc2b7bea8786879e9842d1b4d8d24795f7ff4a143
RELATED_IMAGE_TRILLIAN_LOG_SERVER=registry.redhat.io/rhtas/trillian-logserver-rhel9@sha256:729f2fc77d37d1c45811daddbd1bddd2e5ccf4b9598489a5f728727f84bc2ef7
RELATED_IMAGE_TRILLIAN_DB=registry.redhat.io/rhtas/trillian-database-rhel9@sha256:1295d965ba4f2415742e5b1f858abcac8b03d45708051bc51f28a0e70ce1d417
RELATED_IMAGE_TRILLIAN_NETCAT=registry.redhat.io/openshift4/ose-tools-rhel9@sha256:47eec19e875c3db11a31ccf4c199ef52cf0d2df3b7c424868f55f9e0d0dd43df
RELATED_IMAGE_CREATETREE=registry.redhat.io/rhtas/createtree-rhel9@sha256:bcfc0d077428a5587c8f0cbb4e066684db1bceab06476ffcb5017d153d117269
Expand Down
8 changes: 8 additions & 0 deletions internal/controller/trillian/actions/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,18 @@ const (
MetricsPort = 8090
MetricsPortName = "metrics"

// MySQL
SecretRootPassword = "mysql-root-password"
SecretPassword = "mysql-password"
SecretDatabaseName = "mysql-database"
SecretUser = "mysql-user"
SecretPort = "mysql-port"
SecretHost = "mysql-host"

// PostgreSQL
PgSecretPassword = "postgresql-password"
PgSecretDatabaseName = "postgresql-database"
PgSecretUser = "postgresql-user"
PgSecretPort = "postgresql-port"
PgSecretHost = "postgresql-host"
)
Loading
Loading