allow creating a new operator based off a previous operator#3699
allow creating a new operator based off a previous operator#3699radTuti merged 9 commits intotigera:masterfrom
Conversation
radTuti
force-pushed
the
operator-from
branch
from
4d31d88 to
1073316
Compare
radTuti
force-pushed
the
operator-from
branch
from
1073316 to
8caa076
Compare
danudey
left a comment
danudey
left a comment
There was a problem hiding this comment.
A bit of feedback. I would also request a small README.md with usage examples just to get people up to speed more quickly on how to use the tool and where/when/why.
radTuti
force-pushed
the
operator-from
branch
from
37a302c to
b2d2340
Compare
radTuti
force-pushed
the
operator-from
branch
from
b2d2340 to
6daae8e
Compare
- support custom registry - fixes from validating - modify config files and preserve comments - update examples in README
radTuti
force-pushed
the
operator-from
branch
from
6daae8e to
5c84574
Compare
| require ( | ||
| github.com/corazawaf/coraza-coreruleset v0.0.0-20240226094324-415b1017abdc // indirect | ||
| github.com/magefile/mage v1.14.0 // indirect | ||
| github.com/corazawaf/coraza-coreruleset v0.0.0-20240226094324-415b1017abdc |
There was a problem hiding this comment.
Nit: These 2 require blocks should be merged into the existing ones.
| ``` | ||
|
|
||
| > [!IMPORTANT] | ||
| > To publish the newly created operator, use the `--publish` flag |
There was a problem hiding this comment.
Maybe you can clarify what publish means, from what I understand it is pushing the tag, pushing images + image manifest, but not creating a release in https://github.com/tigera/operator/releases
| // get root directory of operator git repo | ||
| repoRootDir, err := runCommand("git", []string{"rev-parse", "--show-toplevel"}, nil) | ||
| if err != nil { | ||
| return fmt.Errorf("Error getting git root directory: %s", err) |
There was a problem hiding this comment.
For golang users such as myself, there are a lot of warnings. Maybe nice to clean up.
https://go.dev/wiki/CodeReviewComments#error-strings
Error Strings¶
Error strings should not be capitalized (unless beginning with proper nouns or acronyms) or end with > punctuation, since they are usually printed following other context. That is, use fmt.Errorf("something bad") not fmt.Errorf("Something bad"), so that log.Printf("Reading %s: %v", filename, err) formats without a spurious capital letter mid-message. This does not apply to logging, which is implicitly line-oriented and not combined inside other messages.
| if err := encoder.Encode(&root); err != nil { | ||
| return fmt.Errorf("Error encoding updated config: %s", err) | ||
| } | ||
| encoder.Close() |
There was a problem hiding this comment.
You are not handling the potential error
|
|
||
| var exceptEnterpriseFlag = &cli.StringSliceFlag{ | ||
| Name: "except-calico-enterprise", | ||
| Aliases: []string{"except-enterprise", "except-calient"}, |
There was a problem hiding this comment.
The help for this cli is hard to read. I think the number of aliases is part of the reason why. Maybe keep it to 0 or 1 alias?
| var exceptEnterpriseFlag = &cli.StringSliceFlag{ | ||
| Name: "except-calico-enterprise", | ||
| Aliases: []string{"except-enterprise", "except-calient"}, | ||
| Usage: "A list of Enterprise images and the versions to use for them. " + |
There was a problem hiding this comment.
I think this cli tool is not made to handle long usage strings, as a result the help output is hard to read.
radTuti
force-pushed
the
operator-from
branch
from
8b99c92 to
be41c55
Compare
radTuti
force-pushed
the
operator-from
branch
from
2cd0f73 to
158dc66
Compare
4 participants
Description
Allow building an operator image using a previous hashrelease/release as base and changing a few components.
For PR author
If changing pkg/apis/, runmake gen-filesIf changing versions, runmake gen-versionsFor PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bugif this is a bugfix.kind/enhancementif this is a a new feature.enterpriseif this PR applies to Calico Enterprise only.