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

patches/kernelci-pipeline: add timeout to trigger section #99

Closed
wants to merge 1 commit into from

Conversation

JenySadadia
Copy link

The timeout option has been introduced to configure checkout node timeout in trigger service.
Add a default value of the option to staging config as well.

The `timeout` option has been introduced to configure
`checkout` node timeout in `trigger` service.
Add a default value of the option to staging config as well.

Signed-off-by: Jeny Sadadia <[email protected]>
Copy link
Contributor

@gctucker gctucker left a comment

Choose a reason for hiding this comment

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

One issue is that this file isn't used on staging. There's another one which is not in Git and has some secret tokens. We should probably add it as an encrypted file and replace these patches.

@nuclearcat How does that sound from a sysadmin pov?

@nuclearcat
Copy link
Member

nuclearcat commented Sep 14, 2023

I am worried that file contains configuration values that might be changed often and secrets update process are more complicated.
If they are not going to change often it is fine. If they do - then imho we need to separate confidential data from ordinary configuration parameters.

@gctucker
Copy link
Contributor

It shouldn't be changing often, maybe a bit more during intensive development phases but then things should get stable in this respect.

@JenySadadia
Copy link
Author

One issue is that this file isn't used on staging. There's another one which is not in Git and has some secret tokens. We should probably add it as an encrypted file and replace these patches.

@nuclearcat How does that sound from a sysadmin pov?

Yes, I forgot that we are using a different file now. Update the secrets one.

@JenySadadia
Copy link
Author

We should probably add it as an encrypted file and replace these patches.

We can also encrypt specific fields rather than encrypting whole file.

@nuclearcat
Copy link
Member

Should we add it then in kernelci-jenkins-data repo or create new one for API/Pipeline?

@nuclearcat
Copy link
Member

As i see by this repo, initially we just create env file and provide for software (jenkins, but pipeline can do it too) secrets over environment variables.

@gctucker
Copy link
Contributor

I think we can just add an encrypted file to this repo, the Jenkins setup is actually quite clunky.

@JenySadadia With git-crypt you can only encrypt whole files, see how it's setup in kernelci-project for example.

@JenySadadia
Copy link
Author

Closing the PR as we are not using staging.kernelci.org.toml on the staging anymore.
Related to #134

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.

3 participants