Skip to content
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

Use secrets configuration instead of wrapping the secret in action #398

Closed
DnPlas opened this issue Apr 2, 2024 · 2 comments · Fixed by #425
Closed

Use secrets configuration instead of wrapping the secret in action #398

DnPlas opened this issue Apr 2, 2024 · 2 comments · Fixed by #425
Labels
enhancement New feature or request

Comments

@DnPlas
Copy link
Contributor

DnPlas commented Apr 2, 2024

Context

#394 and #397 introduced an action to pass TLS certs and keys to istio-pilot to save them as charm secrets and configure the Ingress Gateway with TLS. On #380 (comment) it is suggested to replace the action with a configuration option and use user secrets instead.

Please note that for this refactor, the desired juju version to use is 3.4, as juju 3.1 does not support user supplied secrets. Please check canonical/bundle-kubeflow#859 for more details on the migration.

What needs to get done

Replace the logic in the charm that observes the action and use a configuration option to pass user secret IDs. This requires us to:

  1. Remove the set-tls and unset-tls actions
  2. Add a configuration option to save user secret IDs
  3. Use the secret ID to retrieve the contents of the secret
  4. Write some unit tests for this

This change has to be done both in main and track/1.17

Definition of Done

The set-tls and unset-tls actions are removed and replaced with a configuration option to pass user secrets.

@DnPlas DnPlas added the enhancement New feature or request label Apr 2, 2024
Copy link

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/KF-5501.

This message was autogenerated

DnPlas added a commit that referenced this issue Apr 2, 2024
The recommended way of handling user secrets is by saving the user secret ID
in a charm configuration, that way the charm can use it to get the contents of the secret.
When the charm is granted with the user secret via the CLI, it allows the charm to observe
secret events, which help handle changes in their contents.
The istio-pilot charm implemented a way to use actions to wrap the secret logic, but because
of the above, this commit does a small refactor to remove the set-tls and unset-tls actions
and instead directly use user secrets.

Part of #398
DnPlas added a commit that referenced this issue Apr 2, 2024
The recommended way of handling user secrets is by saving the user secret ID
in a charm configuration, that way the charm can use it to get the contents of the secret.
When the charm is granted with the user secret via the CLI, it allows the charm to observe
secret events, which help handle changes in their contents.
The istio-pilot charm implemented a way to use actions to wrap the secret logic, but because
of the above, this commit does a small refactor to remove the set-tls and unset-tls actions
and instead directly use user secrets.

This commit also renames ssl to tls to be more accurate when referring
to the actual configuration that gets done in the Gateway.

Part of #398
DnPlas added a commit that referenced this issue Apr 2, 2024
The recommended way of handling user secrets is by saving the user secret ID
in a charm configuration, that way the charm can use it to get the contents of the secret.
When the charm is granted with the user secret via the CLI, it allows the charm to observe
secret events, which help handle changes in their contents.
The istio-pilot charm implemented a way to use actions to wrap the secret logic, but because
of the above, this commit does a small refactor to remove the set-tls and unset-tls actions
and instead directly use user secrets.

This commit also renames ssl to tls to be more accurate when referring
to the actual configuration that gets done in the Gateway.

Part of #398
DnPlas added a commit that referenced this issue Apr 2, 2024
Bumping juju and ops packages to use them in newer versions of the charms,
plus testing them in a CI with a more recent juju version.

This commit also skips some test cases that will be removed in a follow
up commit introduced by #401.

Part of canonical/bundle-kubeflow#859
Part of #398

Signed-off-by: Daniela Plascencia <daniela.plascencia@canonical.com>
DnPlas added a commit that referenced this issue Apr 3, 2024
The recommended way of handling user secrets is by saving the user secret ID
in a charm configuration, that way the charm can use it to get the contents of the secret.
When the charm is granted with the user secret via the CLI, it allows the charm to observe
secret events, which help handle changes in their contents.
The istio-pilot charm implemented a way to use actions to wrap the secret logic, but because
of the above, this commit does a small refactor to remove the set-tls and unset-tls actions
and instead directly use user secrets.

This commit also renames ssl to tls to be more accurate when referring
to the actual configuration that gets done in the Gateway.

Part of #398
DnPlas added a commit that referenced this issue Apr 3, 2024
The recommended way of handling user secrets is by saving the user secret ID
in a charm configuration, that way the charm can use it to get the contents of the secret.
When the charm is granted with the user secret via the CLI, it allows the charm to observe
secret events, which help handle changes in their contents.
The istio-pilot charm implemented a way to use actions to wrap the secret logic, but because
of the above, this commit does a small refactor to remove the set-tls and unset-tls actions
and instead directly use user secrets.

This commit also renames ssl to tls to be more accurate when referring
to the actual configuration that gets done in the Gateway.

Part of #398
@DnPlas
Copy link
Contributor Author

DnPlas commented Apr 3, 2024

An initial approach at solving this issue has been made in #401. Please note some test cases in that PR are skipped because of canonical/operator#1175.

EDIT: The issue for tracking the missing testing bits is canonical/operator#1166, the team has expanded the scope and will include the fix for testing as well as support for the secrets config type.

DnPlas added a commit that referenced this issue Apr 3, 2024
The recommended way of handling user secrets is by saving the user secret ID
in a charm configuration, that way the charm can use it to get the contents of the secret.
When the charm is granted with the user secret via the CLI, it allows the charm to observe
secret events, which help handle changes in their contents.
The istio-pilot charm implemented a way to use actions to wrap the secret logic, but because
of the above, this commit does a small refactor to remove the set-tls and unset-tls actions
and instead directly use user secrets.

This commit also renames ssl to tls to be more accurate when referring
to the actual configuration that gets done in the Gateway.

Part of #398
DnPlas added a commit that referenced this issue May 28, 2024
This commit adds a section in the README to document instructions for
enabling the TLS ingress gateway using juju secrets.

Part of #398
DnPlas added a commit that referenced this issue May 29, 2024
…ress gateway (#401)

* refactor: use secret config instead of action

The recommended way of handling user secrets is by saving the user secret ID
in a charm configuration, that way the charm can use it to get the contents of the secret.
When the charm is granted with the user secret via the CLI, it allows the charm to observe
secret events, which help handle changes in their contents.
The istio-pilot charm implemented a way to use actions to wrap the secret logic, but because
of the above, this commit does a small refactor to remove the set-tls and unset-tls actions
and instead directly use user secrets.

This commit also renames ssl to tls to be more accurate when referring
to the actual configuration that gets done in the Gateway.

Part of #398
DnPlas added a commit that referenced this issue May 29, 2024
This commit adds a section in the README to document instructions for
enabling the TLS ingress gateway using juju secrets.

Part of #398
DnPlas added a commit that referenced this issue May 29, 2024
…ress gateway (#401)

* refactor: use secret config instead of action

The recommended way of handling user secrets is by saving the user secret ID
in a charm configuration, that way the charm can use it to get the contents of the secret.
When the charm is granted with the user secret via the CLI, it allows the charm to observe
secret events, which help handle changes in their contents.
The istio-pilot charm implemented a way to use actions to wrap the secret logic, but because
of the above, this commit does a small refactor to remove the set-tls and unset-tls actions
and instead directly use user secrets.

This commit also renames ssl to tls to be more accurate when referring
to the actual configuration that gets done in the Gateway.

Part of #398
DnPlas added a commit that referenced this issue May 29, 2024
This commit adds a section in the README to document instructions for
enabling the TLS ingress gateway using juju secrets.

Part of #398
DnPlas added a commit that referenced this issue May 29, 2024
…ress gateway (#401)

* refactor: use secret config instead of action

The recommended way of handling user secrets is by saving the user secret ID
in a charm configuration, that way the charm can use it to get the contents of the secret.
When the charm is granted with the user secret via the CLI, it allows the charm to observe
secret events, which help handle changes in their contents.
The istio-pilot charm implemented a way to use actions to wrap the secret logic, but because
of the above, this commit does a small refactor to remove the set-tls and unset-tls actions
and instead directly use user secrets.

This commit also renames ssl to tls to be more accurate when referring
to the actual configuration that gets done in the Gateway.

Part of #398
DnPlas added a commit that referenced this issue May 29, 2024
This commit adds a section in the README to document instructions for
enabling the TLS ingress gateway using juju secrets.

Part of #398
DnPlas added a commit that referenced this issue May 29, 2024
…ress gateway (#401)

* refactor: use secret config instead of action

The recommended way of handling user secrets is by saving the user secret ID
in a charm configuration, that way the charm can use it to get the contents of the secret.
When the charm is granted with the user secret via the CLI, it allows the charm to observe
secret events, which help handle changes in their contents.
The istio-pilot charm implemented a way to use actions to wrap the secret logic, but because
of the above, this commit does a small refactor to remove the set-tls and unset-tls actions
and instead directly use user secrets.

This commit also renames ssl to tls to be more accurate when referring
to the actual configuration that gets done in the Gateway.

Part of #398
DnPlas added a commit that referenced this issue May 29, 2024
This commit adds a section in the README to document instructions for
enabling the TLS ingress gateway using juju secrets.

Part of #398
DnPlas added a commit that referenced this issue May 30, 2024
This PR cherry-picks all the work done for enabling TLS ingress gateway using juju secrets.

* docs: add instructions for TLS with secrets (#421)
* refactor: use secret config instead of action for configuring TLS ingress gateway (#401)
* fix: check routes is not empty before popping values (#424)

Fixes #398
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant