This repository was archived by the owner on Jun 11, 2025. It is now read-only.
feat: add ServiceIntercept CRD and controller for Kubernetes #226
Conversation
This file contains hidden or 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
Reviewer's Guide by SourceryThis PR implements a new ServiceIntercept CRD and controller that enables service interception functionality in Kubernetes clusters. The implementation includes a mutating webhook for pod interception, TLS certificate generation utilities, and the necessary controller logic to manage service intercepts. Class diagram for ServiceIntercept CRDclassDiagram
class ServiceIntercept {
+TypeMeta
+ObjectMeta
+ServiceInterceptSpec Spec
+ServiceInterceptStatus Status
+EnsureGVK()
+GetStatus() rApi.Status
+GetEnsuredLabels() map[string]string
+GetEnsuredAnnotations() map[string]string
}
class ServiceInterceptSpec {
+string ToAddr
+SvcInterceptPortMappings[] PortMappings
}
class ServiceInterceptStatus {
+rApi.Status
+map[string]string Selector
}
class SvcInterceptPortMappings {
+uint16 ContainerPort
+uint16 ServicePort
}
ServiceIntercept --> ServiceInterceptSpec
ServiceIntercept --> ServiceInterceptStatus
ServiceInterceptSpec --> SvcInterceptPortMappings
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
sourcery-ai
bot
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @nxtcoder17 - I've reviewed your changes - here's some feedback:
Overall Comments:
- There's a commented out error return in the TLS cert generation code (networking/internal/gateway/generate-tls-certs.go). This should either be removed or properly handled to avoid masking certificate generation failures.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -42,7 +42,7 @@ func GenTLSCert(dnsNames []string) (caBundle []byte, tlsCert []byte, tlsKey []by | |||
| caCertPEM := new(bytes.Buffer) | |||
| err = pem.Encode(caCertPEM, &pem.Block{Type: "CERTIFICATE", Bytes: caCertBytes}) | |||
There was a problem hiding this comment.
issue (bug_risk): Error from PEM encoding should be properly handled rather than commented out
Ignoring PEM encoding errors could mask serious issues. The error should be propagated to the caller.
| return check.Failed(err) | ||
| } | ||
|
|
||
| for _, p := range podList.Items { |
There was a problem hiding this comment.
issue: Pod deletion handling needs to be more robust against edge cases
The pod deletion logic should handle cases where pods can't be deleted immediately due to network issues or other failures. Consider adding retry logic with timeouts.
| return check.Completed() | ||
| } | ||
|
|
||
| func (r *Reconciler) createSvcIntercept(req *rApi.Request[*crdsv1.ServiceIntercept]) stepResult.Result { |
There was a problem hiding this comment.
issue (complexity): Consider extracting pod creation logic and flattening nested control flow in service intercept methods
The createSvcIntercept and trackInterceptSvc methods could be simplified without losing functionality. Consider:
- Extract pod creation logic into a helper function:
func (r *Reconciler) buildInterceptPod(obj *crdsv1.ServiceIntercept, svc *corev1.Service) (*corev1.Pod, error) {
portMappings := make(map[uint16]uint16, len(obj.Spec.PortMappings))
for _, pm := range obj.Spec.PortMappings {
portMappings[pm.ContainerPort] = pm.ServicePort
}
if obj.Spec.ToAddr == "" {
return nil, fmt.Errorf("no address configured on service intercept")
}
return templates.ParsePodSpec(r.svcInterceptTemplate, map[string]any{
"name": obj.Name + "-intercept",
"namespace": obj.Namespace,
"labels": fn.MapMerge(
map[string]string{
svciGenerationLabel: fmt.Sprintf("%d", obj.Generation),
CreatedForLabel: "intercept",
},
svc.Spec.Selector,
),
"owner-references": []metav1.OwnerReference{fn.AsOwner(obj, true)},
"device-host": obj.Spec.ToAddr,
"port-mappings": portMappings,
})
}- Flatten trackInterceptSvc with early returns:
func (r *Reconciler) trackInterceptSvc(req *rApi.Request[*crdsv1.ServiceIntercept]) stepResult.Result {
ctx, obj := req.Context(), req.Object
check := rApi.NewRunningCheck(TrackInterceptedSvc, req)
svc, err := rApi.Get(ctx, r.Client, fn.NN(obj.Namespace, obj.Name), &corev1.Service{})
if err != nil {
return check.Failed(err)
}
if !fn.IsOwner(obj, fn.AsOwner(svc, true)) {
obj.SetOwnerReferences(append(obj.GetOwnerReferences(), fn.AsOwner(svc, true)))
if err := r.Update(ctx, obj); err != nil {
return check.Failed(err)
}
return check.StillRunning(fmt.Errorf("waiting for reconciliation")).RequeueAfter(1 * time.Second)
}
if err := r.updateSelectorStatus(ctx, obj, svc); err != nil {
return check.StillRunning(err).RequeueAfter(1 * time.Second)
}
return r.cleanupNonInterceptPods(ctx, check, obj, svc)
}These changes improve readability by:
- Separating pod creation logic from state management
- Reducing nesting through early returns
- Breaking complex methods into focused helper functions
nxtcoder17
force-pushed
the
feat/svc-intercept
branch
from
e4f4fb7 to
1855b15
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Summary by Sourcery
Introduce a new ServiceIntercept feature with a custom resource definition and controller to manage service intercepts in Kubernetes. Update kustomization and build configurations to support the new feature.
New Features:
Enhancements:
Build: