Fix env tags on ADO #10902
Merged
Fix env tags on ADO #10902
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
kyma-bot
added
cla: yes
|
/retest |
dekiel
added
the
do-not-merge/hold
kyma-bot
added
size/L
…Encoding should be done for each pipeline call not only from image-builder.
2 tasks
dekiel
removed
the
do-not-merge/hold
akiioto
reviewed
Jun 21, 2024
| decoded, err := base64.StdEncoding.DecodeString(o.tagsBase64) | ||
| if err != nil { | ||
| fmt.Printf("Failed to decode tags, error: %s", err) | ||
| os.Exit(1) |
There was a problem hiding this comment.
Could we return err instead of terminating application?
There was a problem hiding this comment.
Or at least match the previous handling with logger.Errorw
The code appears inconsistent, resembling a mix of different approaches where newer implementations are added without updating the older ones.
There was a problem hiding this comment.
We should return errors instead terminating application and we should use some logging lib instead fmt. But this should be done in whole application. I don't want to do this now, but started replacing in smaller parts when working on some changes. The changes you mention should be done in separate issue and cover whole application. I think such work we should do after removing support for local builds and Prow. The code base to change will be smaller and simpler.
akiioto
approved these changes
Jun 21, 2024
dekiel
added a commit
to dekiel/test-infra
that referenced
this pull request
Jun 21, 2024
* Replace all spaces for ADO Tags parameter * Add support for AzureDevOps CI System * Rename loadProwJobGitState to loadADOGitState * Format double quotes for ADO tags parameter * Use BUILD_BUILDID env variable * Add comment for BUILD_BUILDID env variable * Add comment for beautiful ReplaceAll command * Send only values in Tags parameter * Send only values in Tags parameter * Revert "Send only values in Tags parameter" This reverts commit c8bd817. * Revert "Send only values in Tags parameter" This reverts commit 4c6eb9e. * Revert "Revert "Send only values in Tags parameter"" This reverts commit c1b516a. * Revert "Revert "Send only values in Tags parameter"" This reverts commit 6804bea. * Send only values in Tags parameter * Use double quotes in the test * Encode Tags to base64 * Add --parse-tags-only-base64 * Improve debug message * Improve debug message * Improve debug message * Add additional tag-base64 flag * Fix unit tests * Replace VERSION with GOLANG_VERSION * Support tags passed as comma separated base64 encoded string. * Set fake github action output for dryRun mode. * Fix expected test result. * Encode Tags parameter in pipelines package instead in image-builder. Encoding should be done for each pipeline call not only from image-builder. * Fix test expected value * Set EncodedTags parameter to true when Tags parameter value is not empty. * Add missing expected test value * Remove unused function. --------- Co-authored-by: dekiel <[email protected]>
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.
Description
Changes proposed in this pull request:
Use base64 encoded string as value of Tags parameter in ado api call
Set EncodedTags ado api call parameter to true when using setting Tags parameter value
The Tags parameter in ADO API call is a base64 encoded string. This allows passing special characters like quotes, dotes and slashes without need to escape them.
Because the oci-image-builder pipeline must support encoded and raw Tags parameter value for the time of migration. An additional
EncodedTagsADO API call parameter is set too true to indicate usage of base64. This will be removed once image-builder application will be switched to use encoded Tags parameter.Added
tag-base64flag to accept tags values as base64 stringThe image-builder binary is used in oci-image-builder pipeline to parse tags and pass them to the build step. Because the Tags parameter can contain base64 encoded string, the flag must be used to pass it's value to the image-builder binary.
Use Build.BuildId env variable for determining the Azure DevOps CI System.
This env variable is created by ADO and should be used as a standard way of detecting this CI system in our solutions.
Added dry-run mode with minimal functionality
Added debug mode and logger with structured logging.
Added dry-run mode for testing purposes. This feature was already requested by user and it's implementation should be finished in image-builder github action dry-run #11025
Debug mode and structured logging was added for testing purposes and should be used in whole image-buidler binary replacing existing usage of
fmtpackage functions.