-
Notifications
You must be signed in to change notification settings - Fork 56
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
[WIP] V2 low-level operator implementation #626
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #626 +/- ##
=====================================
Coverage ? 18.98%
=====================================
Files ? 16
Lines ? 3123
Branches ? 0
=====================================
Hits ? 593
Misses ? 2438
Partials ? 92 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing blocking!
linters: | ||
- dupl | ||
- lll | ||
linters: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep this consistent with the linter's we're using in p-k?
} | ||
// Configure authentication strategy to access the source | ||
authData := map[string][]byte{} | ||
authOpts, err := git.NewAuthOptions(*u, authData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So does this just support simple user:password auth?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intend to support the same options, but I was waiting to see how containerization would impact the options. e.g., the ability to mount the git credentials.
# Build | ||
# the GOARCH has not a default value to allow the binary be built according to the host where the command | ||
# was called. For example, if we call make docker-build in a local env which has the Apple Silicon M1 SO | ||
# the docker BUILDPLATFORM arg will be linux/arm64 when for Apple x86 it will be linux/amd64. Therefore, | ||
# by leaving it empty we can ensure that the container and binary shipped on it will have the same platform. | ||
RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -a -o manager cmd/main.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing this is boilerplate? It's a little odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is entirely boilerplate from kubebuilder v.latest.
// if the enable-http2 flag is false (the default), http/2 should be disabled | ||
// due to its vulnerabilities. More specifically, disabling http/2 will | ||
// prevent from being vulnerable to the HTTP/2 Stream Cancelation and | ||
// Rapid Reset CVEs. For more information see: | ||
// - https://github.com/advisories/GHSA-qppj-fm5r-hxr3 | ||
// - https://github.com/advisories/GHSA-4374-p667-p6c8 | ||
disableHTTP2 := func(c *tls.Config) { | ||
setupLog.Info("disabling http/2") | ||
c.NextProtos = []string{"http/1.1"} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't needed anymore - we're using unaffected versions of x/net and grpc.
WebhookServer: webhookServer, | ||
HealthProbeBindAddress: probeAddr, | ||
LeaderElection: enableLeaderElection, | ||
LeaderElectionID: "30fa952a.pulumi.com", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LeaderElectionID: "30fa952a.pulumi.com", | |
LeaderElectionID: "operator.pulumi.com", |
Proposed changes
This PR implements the
auto.pulumi.com
API Group, including theWorkspace
andUpdate
types.Integration tests for the workspace controller are included.
Example
Specific Changes
Environment
specRelated issues (optional)
Closes #619