feat(tibuild): support single platform build

[wuhuizuo]

No description provided.

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Key Changes:

1. Added support for Platform in DevBuildSpec struct in model.go.
2. Updated newDevBuildCloudEvent function in cloud_event_client.go to set the paramPlatform extension based on DevBuildSpec.Platform.
3. Updated validateReq function in dev_build_service.go to check if the platform is supported in the Jenkins engine.

Potential Problems:

1. In the dev_build_service.go file, the import statement for slices seems to be missing. This will cause a build error. The import should be corrected to import "github.com/golang-collections/collections/slices".
2. In the model.go file, the size limit for some string fields like buildEnv, productDockerfile, productBaseImg, builderImg, and githubRepo has been increased to 256 characters, but the size limit for the features field remains at 128 characters. This inconsistency should be addressed to maintain uniformity.
3. The error message in the validateReq function in dev_build_service.go may not provide enough context for the user. It could be improved by providing more specific information about why the platform is not supported in the Jenkins engine.

Fixing Suggestions:

1. Add the import statement for the slices package in the dev_build_service.go file:

   import "github.com/golang-collections/collections/slices"

2. Update the size limit for the features field in the DevBuildSpec struct in model.go to 256 characters to match the other string fields:

   Features string `json:"features,omitempty" gorm:"type:varchar(256)"`

3. Enhance the error message in the validateReq function in dev_build_service.go to provide more specific information:

   return fmt.Errorf("platform %s is not supported in %s engine. Supported platforms for Jenkins engine are: [list of supported platforms]", spec.Platform, spec.PipelineEngine)

By addressing these issues, the pull request will be more consistent, error-free, and provide better user feedback.

[gemini-code-assist]

Hello @wuhuizuo, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request introduces the ability to specify a target platform during the build process. This allows for building images for specific architectures like linux/amd64, linux/arm64, darwin/amd64, or darwin/arm64. The changes involve adding a Platform field to the DevBuildSpec struct, updating the cloud event creation to include the platform, and adding validation to prevent specifying a platform when using the Jenkins pipeline engine. Additionally, some field lengths were increased, and a more efficient way to check for the FIPS feature was implemented.

Highlights

• Platform Support: Adds support for specifying a target platform during the build process.
• Cloud Event Update: Updates the cloud event creation to include the specified platform as an extension.
• Validation: Adds validation to prevent specifying a platform when using the Jenkins pipeline engine.
• FIPS Feature Check: Replaces the loop-based FIPS feature check with slices.Contains for better efficiency.

Changelog

• tibuild/pkg/rest/service/cloud_event_client.go
  • Adds the paramPlatform extension to the cloud event when the Platform field is set in the DevBuildSpec.
• tibuild/pkg/rest/service/dev_build_service.go
  • Imports the slices package.
  • Replaces the loop in hasFIPS function with slices.Contains for checking the presence of FIPS_FEATURE.
  • Adds validation in validateReq to prevent specifying a platform when the pipeline engine is Jenkins.
• tibuild/pkg/rest/service/model.go
  • Adds the Platform field to the DevBuildSpec struct, allowing specification of the target platform.
  • Increases the field lengths of BuildEnv, ProductDockerfile, ProductBaseImg, and TargetImg in the DevBuildSpec struct.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

A platform's choice,
Builds target, finds its voice,
Code's path defined.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The pull request introduces support for building for a single platform, enhancing the flexibility of the tibuild tool. The changes look good overall, but there are a few areas that could be improved for clarity and maintainability.

Summary of Findings

• Error message clarity: The error message in validateReq function could be improved to provide more context about why a platform is not supported in the Jenkins engine. Consider including specific details or a link to documentation.
• String concatenation: In fillForFIPS, consider using fmt.Sprintf for constructing the spec.BuildEnv string to improve readability and maintainability.
• GORM type varchar length: Consider the appropriate length for varchar fields in the DevBuildSpec struct to avoid potential data truncation issues. It's good that some of the varchar lengths were increased, but it's worth reviewing all of them.

Merge Readiness

The pull request is almost ready for merging. Addressing the comments regarding error message clarity and string concatenation would improve the code quality. I am unable to directly approve the pull request, and recommend that others review and approve this code before merging. I recommend that the pull request not be merged until the high severity issues are addressed.

[gemini-code-assist]

This error message is good, but could be improved by providing more context or a link to documentation explaining why platforms are not supported in the Jenkins engine. This will help users understand the limitation and how to address it.

        return fmt.Errorf("platform %s is not supported in %s engine.  See [link to docs] for more information.", spec.Platform, spec.PipelineEngine)

[ti-chi-bot]

@gemini-code-assist[bot]: changing LGTM is restricted to collaborators

In response to this:

Code Review

The pull request introduces support for building for a single platform, enhancing the flexibility of the tibuild tool. The changes look good overall, but there are a few areas that could be improved for clarity and maintainability.

Summary of Findings

• Error message clarity: The error message in validateReq function could be improved to provide more context about why a platform is not supported in the Jenkins engine. Consider including specific details or a link to documentation.
• String concatenation: In fillForFIPS, consider using fmt.Sprintf for constructing the spec.BuildEnv string to improve readability and maintainability.
• GORM type varchar length: Consider the appropriate length for varchar fields in the DevBuildSpec struct to avoid potential data truncation issues. It's good that some of the varchar lengths were increased, but it's worth reviewing all of them.

Merge Readiness

The pull request is almost ready for merging. Addressing the comments regarding error message clarity and string concatenation would improve the code quality. I am unable to directly approve the pull request, and recommend that others review and approve this code before merging. I recommend that the pull request not be merged until the high severity issues are addressed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Key Changes:

1. Added support for specifying a platform in the DevBuildSpec struct.
2. Added logic in newDevBuildCloudEvent function to set the platform extension in the CloudEvent.
3. Added a new import for slices package in dev_build_service.go.
4. Updated the hasFIPS function in dev_build_service.go to use slices.Contains for checking FIPS feature.
5. Added validation in validateReq function in dev_build_service.go to disallow setting platform if the pipeline engine is Jenkins.

Potential Problems:

1. In the validateReq function, the error message for disallowing the platform in Jenkins engine is incorrect. It should mention the platform, not the pipeline engine.
2. The Platform field in DevBuildSpec struct has a comment specifying possible values but it allows an empty string for all platforms. This might cause confusion for users. Consider updating the comment for clarification.

Fixing Suggestions:

1. Update the error message in the validateReq function to mention the platform instead of the pipeline engine. For example:
   Change:

   return fmt.Errorf("can not set platform in %s engine", spec.Platform, spec.PipelineEngine)

   to:

   return fmt.Errorf("cannot set platform when pipeline engine is Jenkins")

2. Update the comment for the Platform field in the DevBuildSpec struct to clarify that an empty string means all platforms. For example:
   Change:

   Platform          string         `json:"platform,omitempty"` // "linux/amd64" or "linux/arm64" or "darwin/amd64" or "darwin/arm64" or empty for all.

   to:

   Platform          string         `json:"platform,omitempty"` // "linux/amd64" or "linux/arm64" or "darwin/amd64" or "darwin/arm64" or empty for all platforms.

Overall, the changes look good, but these suggestions will help improve clarity and prevent potential issues.

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary of Key Changes:

1. Added support for specifying a platform in DevBuildSpec struct.
2. Updated newDevBuildCloudEvent function to include the platform in the CloudEvent if specified.
3. Updated validateReq function in dev_build_service.go to disallow setting a platform when the pipeline engine is Jenkins.
4. Added a new import statement for "slices" in dev_build_service.go for the slices.Contains function.

Potential Problems Identified:

1. Import Statement Issue: The import statement for "slices" in dev_build_service.go seems to be incorrect as there is no standard Go package named slices. It appears to be a custom or third-party package. Make sure the correct package name is used or consider providing the correct import path.

2. Error Handling: In the validateReq function, the error returned when setting a platform while the pipeline engine is Jenkins is not informative. Consider providing a more descriptive error message to help users understand why the action is not allowed.

3. Code Quality: In the hasFIPS function, the logic for checking if a feature contains FIPS could be simplified by using the slices.Contains function directly instead of manually iterating through the features. This can improve code readability and reduce the chance of errors.

Suggestions for Fixes:

1. Import Statement Fix: Check and correct the import statement for "slices" in dev_build_service.go to ensure it points to the correct package or provide the appropriate import path.

2. Error Handling Improvement: Update the error message returned in the validateReq function to provide a clear explanation of why setting a platform with Jenkins as the pipeline engine is not allowed.

3. Code Improvement: Simplify the hasFIPS function by directly using the slices.Contains function to check for the presence of the FIPS feature in the list of features.

By addressing these issues and making the suggested improvements, the pull request should enhance the functionality and maintainability of the codebase.

[wuhuizuo]

/approve

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• tibuild/OWNERS [wuhuizuo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment