Enable users to be able to provide their own config.toml #1171
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pastequo
reviewed
Jul 17, 2024
arewm
force-pushed
the
enable-config-toml-customization
branch
from
032df1a to
9a09a89
Compare
chmeliik
reviewed
Jul 18, 2024
arewm
force-pushed
the
enable-config-toml-customization
branch
from
34a6685 to
7025adb
Compare
chmeliik
reviewed
Jul 22, 2024
pastequo
reviewed
Jul 22, 2024
| @@ -148,6 +154,23 @@ spec: | |||
| echo "$BUILD_DIR" | |||
| ssh -v $SSH_ARGS "$SSH_HOST" mkdir -p "$BUILD_DIR/workspaces" "$BUILD_DIR/scripts" "$BUILD_DIR/tmp" "$BUILD_DIR/tekton-results" "$BUILD_DIR/entitlement" | |||
|
|
|||
| if [! -n "${CONFIG_TOML_FILE}" ] | |||
There was a problem hiding this comment.
Suggested change
| if [! -n "${CONFIG_TOML_FILE}" ] | |
| if [! -n "${CONFIG_TOML_FILE}" ]; then |
chmeliik
reviewed
Jul 22, 2024
Comment on lines
157
to
169
| if [! -n "${CONFIG_TOML_FILE}" ] | ||
| echo "No CONFIG_TOML_FILE specified" | ||
| if [ -f /var/workdir/source/config.toml ]; then | ||
| echo "Using the config.toml file found in the repository root!" | ||
| echo " Remove the config.toml file or set params.CONFIG_TOML_FILE to an invalid path to prevent its use." | ||
| else | ||
| echo "No config.toml file found. Set params.CONFIG_TOML_FILE to use one in the repository." | ||
| export CONFIG_TOML_FILE=config.toml | ||
| fi | ||
| fi | ||
| # ensure that a config toml file is present in case one is not provided or the path is invalid | ||
| mkdir -p /var/workdir/source/$(dirname $CONFIG_TOML_FILE) | ||
| echo "Using the following config.toml file:" | ||
| cat /var/workdir/source/$CONFIG_TOML_FILE |
There was a problem hiding this comment.
Desired behavior:
| config path | not specified (defaults to config.toml) | specified |
|---|---|---|
| exists | config.toml used | specified path used |
| does not exist | empty conf used | error |
Current behavior:
| config path | not specified (defaults to config.toml) | specified |
|---|---|---|
| exists | config.toml used ✔️ | specified path used ✔️ |
| does not exist | error ❗ | error ✔️ |
chmeliik
reviewed
Jul 22, 2024
arewm
force-pushed
the
enable-config-toml-customization
branch
2 times, most recently
from
83745c2 to
5c30601
Compare
chmeliik
approved these changes
Jul 22, 2024
This removes the default configuration (which also removes a default user and password from being injected into all images), instead deferring to the config.toml which is present in the source repository. If an invalid config file is provided, the build will fail. Otherwise, the task will use a config.toml file if it is present in the repository root or it will create an empty config file to use. Signed-off-by: arewm <[email protected]>
arewm
force-pushed
the
enable-config-toml-customization
branch
from
5c30601 to
c8f32df
Compare
|
/retest |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
https://github.com/konflux-ci/build-definitions/pull/1165/checks?check_run_id=2785204219 I thinks that this PR causes that all PRs are failing on linter error ^, I don't know why this one passed the test though IMO this is the faulty line and linter makes it as false positive report |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the default configuration (which also removes a default user and password from being injected into all images), instead deferring to the config.toml which is present in the source repository.
Before you complete this pull request ...
Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.