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

fix(Konflux 5917): add an example how user can use a secret in pipeline #208

Merged
merged 3 commits into from
Feb 5, 2025

Conversation

kasemAlem
Copy link

@kasemAlem kasemAlem commented Jan 15, 2025

We added a tekton task as an example how user can use the created secrets
within a build or integration pipeline

KONFLUX-5917

@kasemAlem kasemAlem changed the title Konflux 5917 fix(Konflux 5917): add an example how user can use a secret in pipeline Jan 15, 2025
Copy link

🚀 Preview is available at https://pr-208--konflux-docs.netlify.app

Comment on lines 68 to 70
- name: registry-credentials
secret:
secretName: registry.redhat.io
Copy link
Member

Choose a reason for hiding this comment

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

This is how you use a secret, but what does this secret look like that you are using? Is it clear how values need to match up between the secret and its use?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. I think only one secret name in volumes might not enough as one example.

Copy link
Author

Choose a reason for hiding this comment

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

I'm confused, maybe I'm missing something
In the current documentation of creating secrets it's detailed step by step how user can create a secret of each type, also there is a good example stated in a note frame :

One such task is the [sast-snyk-check](https://github.com/konflux-ci/build-definitions/tree/main/task/sast-snyk-check) task that uses the third-party service [snyk](https://snyk.io/) to perform static application security testing (SAST) as a part of the default {ProductName} pipeline. Use this procedure to upload your snyk.io token. Name the secret snyk-secret so that the snyk task in the {ProductName} pipeline will recognize it and use it.

It's an example how a third party tool in task requires a specific secret and how it's been used within the task , isn't ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would providing a link to the Creating registry pull secrets section of the guide either above or below this example do the job of clarifying how the secret should look like?

Copy link
Author

Choose a reason for hiding this comment

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

@arewm ^^

Copy link
Member

Choose a reason for hiding this comment

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

What does the secret CR need to look like? Maybe this is just a lack of knowledge/experience coming from me. Maybe this is documented well for k8s.

What I understand here is that you are describing use of a secret by its name, but are there specific properties of how that secret is created in k8s that need to be reflected in the task?

Also, for your awareness, in #214 we have started to link the different places that secrets are documented in this page.

Copy link
Author

@kasemAlem kasemAlem Jan 28, 2025

Choose a reason for hiding this comment

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

Thanks @arewm , hope the new fix can be clear for a user :

Screenshot from 2025-01-28 20-22-12

Copy link

🚀 Preview is available at https://pr-208--konflux-docs.netlify.app

@@ -30,13 +30,6 @@ NOTE: One such task is the link:https://github.com/konflux-ci/build-definitions/
. Optional: Under **Labels**, add a label to tag or provide more context for your secret.
. Click **Add secret**.

=== Notable task input secrets
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to delete this?

Copy link
Author

@kasemAlem kasemAlem Jan 29, 2025

Choose a reason for hiding this comment

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

was deleted by mistake , returned
thanks

@@ -54,6 +47,61 @@ Some container builds may use parent images from registries that require authent
. Enter the password for the registry in **Password**.
. Click **Add secret**.

== Example of creating a quay.io secret
Copy link
Member

Choose a reason for hiding this comment

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

Is this a bad example? Can users not get access to a registry secret without this additional task changes? When adding the secret with the UI, we link it with the ServiceAccount and it is then available in the task.

We do not, for example, do this in the buildah tasks: https://github.com/konflux-ci/build-definitions/blob/main/task/buildah-oci-ta/0.3/buildah-oci-ta.yaml#L159-L184

Copy link
Author

Choose a reason for hiding this comment

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

you are right,
the fact is that secrets created via UI are automatically linked to the SA
so no action is required , I've removed the task example and updated the note
@arewm . WDYT?

Copy link

🚀 Preview is available at https://pr-208--konflux-docs.netlify.app

@kasemAlem kasemAlem force-pushed the KONFLUX-5917 branch 3 times, most recently from bebbc53 to df60879 Compare January 29, 2025 14:03
Copy link

🚀 Preview is available at https://pr-208--konflux-docs.netlify.app

Comment on lines 93 to 94
* **Manual Secret Creation**
If you create the secret manually (e.g., via `kubectl` or YAML), you must **manually link it** to the `appstudio-pipeline` ServiceAccount in your namespace.
Copy link
Member

Choose a reason for hiding this comment

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

Do secrets always need to be linked to the ServiceAccount in order for it to be used by a task?

Copy link
Author

Choose a reason for hiding this comment

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

another options :

  1. Create secret is in namespace and just have to make sure SA got the right access permissions by Role/RoleBinding
  2. Secret as Env environment

let me add both ways

Copy link
Author

Choose a reason for hiding this comment

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

fixed

Copy link

github-actions bot commented Feb 5, 2025

🚀 Preview is available at https://pr-208--konflux-docs.netlify.app

Copy link
Contributor

@dirgim dirgim left a comment

Choose a reason for hiding this comment

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

LGTM


please review the xref:/troubleshooting/index.adoc#check-if-the-secret-is-linked-to-the-service-account[troubleshooting section]) for more info.
* **Automatic Secret Linking via UI**
The Konflux UI automatically links image pull secrets to the `appstudio-pipeline` ServiceAccount.
Copy link
Member

Choose a reason for hiding this comment

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

Last change before we can merge, please use {ProductName} instead of Konflux for consistency.

Copy link
Author

@kasemAlem kasemAlem Feb 5, 2025

Choose a reason for hiding this comment

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

nice catch,
fixed

Copy link

github-actions bot commented Feb 5, 2025

🚀 Preview is available at https://pr-208--konflux-docs.netlify.app

Copy link
Member

@arewm arewm left a comment

Choose a reason for hiding this comment

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

thanks!

@arewm arewm merged commit a308f2d into konflux-ci:main Feb 5, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants