-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add support for Linkerd to the intents-operator #324
base: main
Are you sure you want to change the base?
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
Over the upcoming period there's still some extra small functionality i want to add to the PR, but i've created it now to work on any large changes required at the same time Including the following:
|
} | ||
|
||
pod, err := r.serviceIdResolver.ResolveClientIntentToPod(ctx, *intents) | ||
logrus.Infof("Got pod with name %s", pod.Name) |
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.
Move this log after the err check, as pod may be nil
logrus.Infof("Reconciling Linkerd authorization policies for service %s in namespace %s", | ||
intents.Spec.Service.Name, req.Namespace) | ||
|
||
// TODO: implement delete all |
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.
Is this TODO still relevant?
} | ||
|
||
clientServiceAccountName := pod.Spec.ServiceAccountName | ||
missingSideCar := !linkerdmanager.IsPodPartOfLinkerdMesh(pod) |
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.
Is there a way to check using a label or annotation instead?
Using the proxy's name is implementation-specific and may break when Linkerd versions are updated. Is there a documented label or something similar we can check for instead?
return ctrl.Result{}, nil | ||
} | ||
|
||
err = r.linkerdManager.Create(ctx, intents, clientServiceAccountName) // the injectable recorder is null pointer |
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.
comment still relevant?
src/shared/operatorconfig/config.go
Outdated
EnableDatabasePolicy = "enable-database-policy-creation" // Whether to enable the new database reconciler | ||
EnableDatabasePolicyDefault = true | ||
EnableDatabaseReconciler = "enable-database-reconciler" // Whether to enable the new database reconciler | ||
EnableDatabaseReconcilerDefault = false |
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.
change this back :)
@@ -0,0 +1,285 @@ | |||
apiVersion: apiextensions.k8s.io/v1 |
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.
Please remove these generated .patched files from Git
@@ -155,6 +157,9 @@ type Intent struct { | |||
//+optional | |||
Type IntentType `json:"type,omitempty" yaml:"type,omitempty"` | |||
|
|||
//+optional | |||
Port int32 `json:"port,omitempty" yaml:"port,omitempty"` |
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.
We still need to figure out a way to do without specifying the port manually - let's try to figure this out when we talk
func (r *LinkerdReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { | ||
logrus.Info("Starting linkerd reconcile logic") | ||
|
||
installed, err := linkerdmanager.IsLinkerdServerInstalled(ctx, r.Client) |
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.
installed, err := linkerdmanager.IsLinkerdServerInstalled(ctx, r.Client) | |
installed, err := linkerdmanager.IsLinkerdInstalled(ctx, r.Client) |
This checks if the Linkerd Server CRD is installed, but in fact it's checking if Linkerd itself is installed in the cluster through this check.
if intent.Type != "" && intent.Type != otterizev1alpha3.IntentTypeHTTP && intent.Port != 0 { // this will skip non http ones, db for example, skip port doesnt exist as well | ||
continue | ||
} | ||
shouldCreatePolicy, err := protected_services.IsServerEnforcementEnabledDueToProtectionOrDefaultState( //TODO: check what that does |
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 is enabled through ProtectedService: https://docs.otterize.com/shadow-vs-active-enforcement#protected-services
} | ||
|
||
for _, intent := range clientIntents.GetCallsList() { | ||
if intent.Type != "" && intent.Type != otterizev1alpha3.IntentTypeHTTP && intent.Port != 0 { // this will skip non http ones, db for example, skip port doesnt exist as well |
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.
The port condition seems inverted, as in if port is NOT empty AND type is not HTTP it will continue. But perhaps you meant
if intent.Type != "" // if intent type is not empty
&& intent.Type != IntentTypeHTTP // and intent type is not http
|| intent.Port == "" // OR port is empty
we'd be very interested in having support for Linkerd2 - then Otterize would really be a compelling solution for us as light user of EKS & Linkerd2 (mTLS, grpc load balancing but not leveraging authorization policies in linkerd). We use kubernetes network policies but we do not like it for the same reason you highlighted in your blog post. We're making a case towards dropping support for network policies all together (we are using Calico & aws vpc-cni) and we would be interesting in assessing how linkerd2/otterize could help with authorization policies on that side of the fence instead. |
That's great to hear @aquam8! We've been working on a couple of infrastructure changes that are starting to come in and have blocked this PR from being merged, namely #409 and #439, so Linkerd support should be getting merged soon! Stay tuned :) Would love to have you join us on Slack to hear more about your use case if possible: https://joinslack.otterize.com |
Could this work be revised now that the dependent PRs have been merged, thank you! |
@evyatarmeged @orishoshan |
This package contains an engine that creates, updates, deletes linkerd resources from ClientIntents. With a test suite for that feature based on mock. The package will create a Server, AuthPolicy and MTLSAuth for a non HTTP Intent, or a Server ,HTTPRoute , AuthPolicy and MTLSAuth for a HTTP Intent. As well as a NetAuth for probe routes to not cause probes to break from the creation of servers on them. Signed-off-by: aerosouund <[email protected]>
Create Linkerd mock to be used in testing Signed-off-by: aerosouund <[email protected]>
Leverage the linkerd engine in a reconciler that watches ClientIntents Signed-off-by: aerosouund <[email protected]>
Register the linkerd reconciler with the intents reconciler Signed-off-by: aerosouund <[email protected]>
Define constants used by the linkerd manager Signed-off-by: aerosouund <[email protected]>
Signed-off-by: aerosouund <[email protected]>
This is needed as pod ports are required for Linkerd Server creation and they might change if the pod definition changes. Signed-off-by: aerosouund <[email protected]>
…n pod reconciler Signed-off-by: aerosouund <[email protected]>
Signed-off-by: aerosouund <[email protected]>
Description
Create required linkerd resources (servers, httproutes, authpolicies, mtls authentications, network authentications) based on otterize client intents.
An extra optional field has been added in the intent definition which is a port field.
It extends the same pattern used in istio policies but applies it to each respective linkerd resource
References
Testing
There is a built image in a public repo that you can deploy in a cluster with linkerd installed:
aerosouund/intents-operator:0.1.34
Also include details of the environment this PR was developed in (language/platform/browser version):
golang 1.21
kubernetes 1.23 on EKS
Linkerd 2.x
This change adds test coverage for new/changed/fixed functionality
Checklist