diff --git a/.github/workflow-helpers/kind-intents.yaml b/.github/workflow-helpers/kind-intents.yaml new file mode 100644 index 000000000..a91e7dab4 --- /dev/null +++ b/.github/workflow-helpers/kind-intents.yaml @@ -0,0 +1,12 @@ +apiVersion: k8s.otterize.com/v1alpha3 +kind: ClientIntents +metadata: + name: client + namespace: otterize-tutorial-npol +spec: + service: + kind: Deployment + name: client + calls: + - name: server + kind: Service \ No newline at end of file diff --git a/.github/workflows/netpol-e2e-test.yaml b/.github/workflows/netpol-e2e-test.yaml index c2e52a327..45d46657e 100644 --- a/.github/workflows/netpol-e2e-test.yaml +++ b/.github/workflows/netpol-e2e-test.yaml @@ -372,11 +372,140 @@ jobs: + + e2e-test-intents-with-kind-after-pods-with-egress: + timeout-minutes: 10 + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + with: + submodules: recursive + + - name: Start minikube + uses: medyagh/setup-minikube@master + with: + start-args: "--network-plugin=cni --cni=calico" + + - name: Load images from GitHub Artifacts + if: github.repository != 'otterize/intents-operator' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != 'otterize/intents-operator') + uses: actions/download-artifact@v3 + with: + name: ${{ env.REGISTRY }}_${{ github.actor }}_intents-operator_${{ github.sha }}.tar + + - name: Load Docker image + if: github.repository != 'otterize/intents-operator' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != 'otterize/intents-operator') + run: |- + docker image load -i intents-operator.tar + minikube image load ${{ env.REGISTRY }}/${{ github.actor }}/intents-operator:${{ github.sha }} + + - name: Login to GCR + if: (github.event_name == 'push' && github.repository == 'otterize/intents-operator') || github.event.pull_request.head.repo.full_name == 'otterize/intents-operator' + uses: docker/login-action@v2 + with: + registry: ${{ env.REGISTRY }} + username: _json_key_base64 + password: ${{ secrets.B64_GCLOUD_SERVICE_ACCOUNT_JSON}} + + - name: Load Docker images from GCR + if: (github.event_name == 'push' && github.repository == 'otterize/intents-operator') || github.event.pull_request.head.repo.full_name == 'otterize/intents-operator' + run: |- + docker pull ${{ env.REGISTRY }}/intents-operator:${{ inputs.operator-tag }} + minikube image load ${{ env.REGISTRY }}/intents-operator:${{ inputs.operator-tag }} + + - name: Set up Helm + uses: azure/setup-helm@v3 + + - name: Wait for Calico startup + run: |- + kubectl wait pods -n kube-system -l k8s-app=calico-kube-controllers --for condition=Ready --timeout=90s + kubectl wait pods -n kube-system -l k8s-app=calico-node --for condition=Ready --timeout=90s + kubectl wait pods -n kube-system -l k8s-app=calico-kube-controllers --for condition=Ready --timeout=90s + + - name: Install Otterize + run: |- + OPERATOR_FLAGS="--set-string intentsOperator.operator.repository=${{ env.REGISTRY }} --set-string intentsOperator.operator.image=${{ inputs.operator-image }} --set-string intentsOperator.operator.tag=${{ inputs.operator-tag }} --set-string intentsOperator.operator.pullPolicy=Never" + TELEMETRY_FLAG="--set global.telemetry.enabled=false" + EGRESS_FLAG="--set intentsOperator.operator.enableEgressNetworkPolicyCreation=true" + helm dep up ./helm-charts/otterize-kubernetes + helm install otterize ./helm-charts/otterize-kubernetes -n otterize-system --create-namespace $OPERATOR_FLAGS $TELEMETRY_FLAG $EGRESS_FLAG + + + - name: Deploy Tutorial services + run: |- + kubectl apply -f https://docs.otterize.com/code-examples/automate-network-policies/all.yaml + + - name: Wait for Otterize + run: |- + kubectl wait pods -n otterize-system -l app=intents-operator --for condition=Ready --timeout=360s + # wait for webhook to be ready + POD_IP=`kubectl get pod -l app=intents-operator -n otterize-system -o=jsonpath='{.items[0].status.podIP}'` + kubectl wait -n otterize-system --for=jsonpath='{.subsets[0].addresses[0].ip}'=$POD_IP endpoints/intents-operator-webhook-service + # wait for CRD update + kubectl wait --for=jsonpath='{.spec.conversion.webhook.clientConfig.service.namespace}'=otterize-system customresourcedefinitions/clientintents.k8s.otterize.com + + + - name: Wait for Tutorial services + run: |- + kubectl wait pods -n otterize-tutorial-npol -l app=client --for condition=Ready --timeout=180s + kubectl wait pods -n otterize-tutorial-npol -l app=client-other --for condition=Ready --timeout=180s + kubectl wait pods -n otterize-tutorial-npol -l app=server --for condition=Ready --timeout=180s + + - name: Before apply intents + run: |- + CLI1_POD=`kubectl get pod --selector app=client -n otterize-tutorial-npol -o json | jq -r ".items[0].metadata.name"` + CLI2_POD=`kubectl get pod --selector app=client-other -n otterize-tutorial-npol -o json | jq -r ".items[0].metadata.name"` + echo Client: $CLI1_POD client_other: $CLI2_POD + source .github/workflows/test-bashrc.sh + + # using 14 because the log repeat itself every 14 lines + echo check client log + wait_for_log $CLI1_POD 10 "Hi, I am the server, you called, may I help you?" + + echo check client other log + wait_for_log $CLI2_POD 10 "Hi, I am the server, you called, may I help you?" + + - name: Apply intents and test connectivity + run: |- + CLI1_POD=`kubectl get pod --selector app=client -n otterize-tutorial-npol -o json | jq -r ".items[0].metadata.name"` + CLI2_POD=`kubectl get pod --selector app=client-other -n otterize-tutorial-npol -o json | jq -r ".items[0].metadata.name"` + echo Client: $CLI1_POD client_other: $CLI2_POD + source .github/workflows/test-bashrc.sh + + echo "Apply intents" + apply_intents_and_wait_for_webhook ./.github/workflow-helpers/kind-intents.yaml + echo "Intents applied" + + # should not work at first because there is no allow DNS netpol + echo "check client log - should get timed out because it is missing DNS allow netpol" + wait_for_log $CLI1_POD 10 "curl timed out" + + # should be blocked (using 3 because the log should repeat itself every 3 lines) + echo "check client other log - should get timed out because it does not have an applied intent" + wait_for_log $CLI2_POD 10 "curl timed out" + + echo "apply allow DNS netpol" + kubectl apply -f .github/workflow-helpers/allowDNS.yaml -n otterize-tutorial-npol + + # should work because there is an applied intent and allowDNS netpol + echo "check client log - should work because there is an applied intent and allowDNS netpol" + wait_for_log $CLI1_POD 10 "Hi, I am the server, you called, may I help you?" + + # should be blocked (using 3 because the log should repeat itself every 3 lines) + echo "check client other log - should get timed out because it does not have an applied intent" + wait_for_log $CLI2_POD 10 "curl timed out" + + + + + + e2e-test: needs: - e2e-test-intents-after-pods - e2e-test-intents-before-pods - e2e-test-intents-after-pods-with-egress + - e2e-test-intents-with-kind-after-pods-with-egress runs-on: ubuntu-latest steps: - run: |- diff --git a/src/operator/Makefile b/src/operator/Makefile index c1db8cc22..deed32541 100644 --- a/src/operator/Makefile +++ b/src/operator/Makefile @@ -138,7 +138,8 @@ deploy: manifests copy-manifests-to-helm helm-dependency deploy-local: manifests copy-manifests-to-helm helm-dependency ## Deploy images built locally into the cluster. helm upgrade --install -n otterize-system --create-namespace otterize $(OTTERIZE_HELM_CHART_DIR) \ --set intentsOperator.operator.tag=$(LOCAL_IMAGE_TAG) \ - --set intentsOperator.operator.pullPolicy=Never + --set intentsOperator.operator.pullPolicy=Never \ + --set global.telemetry.enabled=false .PHONY: undeploy undeploy: ## Undeploy controller from the K8s cluster specified in ~/.kube/config. Call with ignore-not-found=true to ignore resource not found errors during deletion. diff --git a/src/operator/api/v1alpha3/clientintents_types.go b/src/operator/api/v1alpha3/clientintents_types.go index 8a4c3e3c1..fd164363a 100644 --- a/src/operator/api/v1alpha3/clientintents_types.go +++ b/src/operator/api/v1alpha3/clientintents_types.go @@ -17,11 +17,10 @@ limitations under the License. package v1alpha3 import ( - "crypto/md5" - "encoding/hex" "encoding/json" "fmt" "github.com/otterize/intents-operator/src/shared/errors" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "strconv" "strings" @@ -29,7 +28,6 @@ import ( "github.com/samber/lo" "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/sets" ) @@ -37,45 +35,46 @@ import ( // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. const ( - OtterizeAccessLabelPrefix = "intents.otterize.com/access" - OtterizeServiceAccessLabelPrefix = "intents.otterize.com/svc-access" - OtterizeAccessLabelKey = "intents.otterize.com/access-%s" - OtterizeSvcAccessLabelKey = "intents.otterize.com/svc-access-%s" - OtterizeClientLabelKey = "intents.otterize.com/client" - OtterizeServiceLabelKey = "intents.otterize.com/service" - OtterizeServerLabelKeyDeprecated = "intents.otterize.com/server" - OtterizeKubernetesServiceLabelKeyPrefix = "intents.otterize.com/k8s-svc" - OtterizeKubernetesServiceLabelKey = "intents.otterize.com/k8s-svc-%s" - KubernetesStandardNamespaceNameLabelKey = "kubernetes.io/metadata.name" - AllIntentsRemovedAnnotation = "intents.otterize.com/all-intents-removed" - OtterizeCreatedForServiceAnnotation = "intents.otterize.com/created-for-service" - OtterizeCreatedForIngressAnnotation = "intents.otterize.com/created-for-ingress" - OtterizeSingleNetworkPolicyNameTemplate = "%s-access" - OtterizeNetworkPolicy = "intents.otterize.com/network-policy" - OtterizeSvcNetworkPolicy = "intents.otterize.com/svc-network-policy" - OtterizeNetworkPolicyServiceDefaultDeny = "intents.otterize.com/network-policy-service-default-deny" - OtterizeNetworkPolicyExternalTraffic = "intents.otterize.com/network-policy-external-traffic" - ClientIntentsFinalizerName = "intents.otterize.com/client-intents-finalizer" - ProtectedServicesFinalizerName = "intents.otterize.com/protected-services-finalizer" - OtterizeIstioClientAnnotationKey = "intents.otterize.com/istio-client" - OtterizeClientServiceAccountAnnotation = "intents.otterize.com/client-intents-service-account" - OtterizeSharedServiceAccountAnnotation = "intents.otterize.com/shared-service-account" - OtterizeMissingSidecarAnnotation = "intents.otterize.com/service-missing-sidecar" - OtterizeServersWithoutSidecarAnnotation = "intents.otterize.com/servers-without-sidecar" - OtterizeTargetServerIndexField = "spec.service.calls.server" - OtterizeKafkaServerConfigServiceNameField = "spec.service.name" - OtterizeProtectedServiceNameIndexField = "spec.name" - OtterizeFormattedTargetServerIndexField = "formattedTargetServer" - EndpointsPodNamesIndexField = "endpointsPodNames" - IngressServiceNamesIndexField = "ingressServiceNames" - MaxOtterizeNameLength = 20 - MaxNamespaceLength = 20 - OtterizeSvcEgressNetworkPolicy = "intents.otterize.com/svc-egress-network-policy" - OtterizeEgressNetworkPolicy = "intents.otterize.com/egress-network-policy" - OtterizeInternetNetworkPolicy = "intents.otterize.com/egress-internet-network-policy" - OtterizeInternetTargetName = "internet" - KubernetesAPIServerName = "kubernetes" - KubernetesAPIServerNamespace = "default" + OtterizeAccessLabelPrefix = "intents.otterize.com/access" + OtterizeServiceAccessLabelPrefix = "intents.otterize.com/svc-access" + OtterizeAccessLabelKey = "intents.otterize.com/access-%s" + OtterizeSvcAccessLabelKey = "intents.otterize.com/svc-access-%s" + OtterizeClientLabelKey = "intents.otterize.com/client" + OtterizeServiceLabelKey = "intents.otterize.com/service" + OtterizeOwnerKindLabelKey = "intents.otterize.com/owner-kind" + OtterizeServerLabelKeyDeprecated = "intents.otterize.com/server" + KubernetesStandardNamespaceNameLabelKey = "kubernetes.io/metadata.name" + AllIntentsRemovedAnnotation = "intents.otterize.com/all-intents-removed" + OtterizeCreatedForServiceAnnotation = "intents.otterize.com/created-for-service" + OtterizeCreatedForIngressAnnotation = "intents.otterize.com/created-for-ingress" + OtterizeSingleNetworkPolicyNameTemplate = "%s-access" + OtterizeNetworkPolicy = "intents.otterize.com/network-policy" + OtterizeSvcNetworkPolicy = "intents.otterize.com/svc-network-policy" + OtterizeNetworkPolicyServiceDefaultDeny = "intents.otterize.com/network-policy-service-default-deny" + OtterizeNetworkPolicyExternalTraffic = "intents.otterize.com/network-policy-external-traffic" + ClientIntentsFinalizerName = "intents.otterize.com/client-intents-finalizer" + ProtectedServicesFinalizerName = "intents.otterize.com/protected-services-finalizer" + OtterizeIstioClientAnnotationKeyDeprecated = "intents.otterize.com/istio-client" + OtterizeIstioClientWithKindLabelKey = "intents.otterize.com/istio-client-with-kind" + OtterizeClientServiceAccountAnnotation = "intents.otterize.com/client-intents-service-account" + OtterizeSharedServiceAccountAnnotation = "intents.otterize.com/shared-service-account" + OtterizeMissingSidecarAnnotation = "intents.otterize.com/service-missing-sidecar" + OtterizeServersWithoutSidecarAnnotation = "intents.otterize.com/servers-without-sidecar" + OtterizeTargetServerIndexField = "spec.service.calls.server" + OtterizeKafkaServerConfigServiceNameField = "spec.service.name" + OtterizeProtectedServiceNameIndexField = "spec.name" + OtterizeFormattedTargetServerIndexField = "formattedTargetServer" + OtterizePodByOwnerKindAndNameIndexField = "podByOwnerKindAndName" + EndpointsPodNamesIndexField = "endpointsPodNames" + IngressServiceNamesIndexField = "ingressServiceNames" + MaxOtterizeNameLength = 20 + MaxNamespaceLength = 20 + OtterizeSvcEgressNetworkPolicy = "intents.otterize.com/svc-egress-network-policy" + OtterizeEgressNetworkPolicy = "intents.otterize.com/egress-network-policy" + OtterizeInternetNetworkPolicy = "intents.otterize.com/egress-internet-network-policy" + OtterizeInternetTargetName = "internet" + KubernetesAPIServerName = "kubernetes" + KubernetesAPIServerNamespace = "default" ) // +kubebuilder:validation:Enum=http;kafka;database;aws;gcp;azure;internet @@ -227,12 +226,17 @@ type IntentsSpec struct { type Service struct { Name string `json:"name" yaml:"name"` + //+optional + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` } type Intent struct { //+optional Name string `json:"name,omitempty" yaml:"name,omitempty"` + //+optional + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` + //+optional Type IntentType `json:"type,omitempty" yaml:"type,omitempty"` @@ -357,6 +361,13 @@ func (in *ClientIntents) GetFilteredCallsList(intentTypes ...IntentType) []Inten }) } +func (in *ClientIntents) GetClientKind() string { + if in.Spec.Service.Kind == "" { + return serviceidentity.KindOtterizeLegacy + } + return in.Spec.Service.Kind +} + func (in *ClientIntents) GetIntentsLabelMapping(requestNamespace string) map[string]string { otterizeAccessLabels := make(map[string]string) @@ -364,10 +375,10 @@ func (in *ClientIntents) GetIntentsLabelMapping(requestNamespace string) map[str if intent.Type == IntentTypeAWS || intent.Type == IntentTypeGCP || intent.Type == IntentTypeAzure || intent.Type == IntentTypeDatabase { continue } - ns := intent.GetTargetServerNamespace(requestNamespace) - labelKey := fmt.Sprintf(OtterizeAccessLabelKey, GetFormattedOtterizeIdentity(intent.GetTargetServerName(), ns)) + targetServiceIdentity := intent.ToServiceIdentity(requestNamespace) + labelKey := fmt.Sprintf(OtterizeAccessLabelKey, targetServiceIdentity.GetFormattedOtterizeIdentityWithKind()) if intent.IsTargetServerKubernetesService() { - labelKey = fmt.Sprintf(OtterizeSvcAccessLabelKey, GetFormattedOtterizeIdentity("svc."+intent.GetTargetServerName(), ns)) + labelKey = fmt.Sprintf(OtterizeSvcAccessLabelKey, targetServiceIdentity.GetFormattedOtterizeIdentityWithKind()) } otterizeAccessLabels[labelKey] = "true" } @@ -402,7 +413,7 @@ func (in *Intent) GetTargetServerNamespace(intentsObjNamespace string) string { } func (in *Intent) IsTargetServerKubernetesService() bool { - return strings.HasPrefix(in.Name, "svc:") + return strings.HasPrefix(in.Name, "svc:") || in.Kind == serviceidentity.KindService } func (in *Intent) IsTargetTheKubernetesAPIServer(objectNamespace string) bool { @@ -447,6 +458,16 @@ func (in *Intent) GetTargetServerName() string { } } +func (in *Intent) GetTargetServerKind() string { + if in.Kind != "" { + return in.Kind + } + if in.IsTargetServerKubernetesService() { + return serviceidentity.KindService + } + return serviceidentity.KindOtterizeLegacy +} + func (in *Intent) GetServerFullyQualifiedName(intentsObjNamespace string) string { fullyQualifiedName := fmt.Sprintf("%s.%s", in.GetTargetServerName(), in.GetTargetServerNamespace(intentsObjNamespace)) return fullyQualifiedName @@ -506,8 +527,9 @@ func (in *ClientIntents) IsServerMissingSidecar(intent Intent) (bool, error) { if err != nil { return false, errors.Wrap(err) } - serverIdentity := GetFormattedOtterizeIdentity(intent.GetTargetServerName(), intent.GetTargetServerNamespace(in.Namespace)) - return serversSet.Has(serverIdentity), nil + serviceIdentity := intent.ToServiceIdentity(in.Namespace) + formattedServerIdentity := serviceIdentity.GetFormattedOtterizeIdentityWithoutKind() + return serversSet.Has(formattedServerIdentity), nil } func (in *ClientIntentsList) FormatAsOtterizeIntents() ([]*graphqlclient.IntentInput, error) { @@ -727,44 +749,6 @@ func intentsHTTPResourceToCloud(resource HTTPResource, index int) *graphqlclient return &httpConfig } -// GetFormattedOtterizeIdentity truncates names and namespaces to a 20 char len string (if required) -// It also adds a short md5 hash of the full name+ns string and returns the formatted string -// This is due to Kubernetes' limit on 63 char label keys/values -func GetFormattedOtterizeIdentity(name, ns string) string { - // Get MD5 for full length "name-namespace" string - hash := md5.Sum([]byte(fmt.Sprintf("%s-%s", name, ns))) - - // Truncate name and namespace to 20 chars each - if len(name) > MaxOtterizeNameLength { - name = name[:MaxOtterizeNameLength] - } - - if len(ns) > MaxNamespaceLength { - ns = ns[:MaxNamespaceLength] - } - // A 6 char hash, even though truncated, leaves 2 ^ 48 combinations which should be enough - // for unique identities in a k8s cluster - hashSuffix := hex.EncodeToString(hash[:])[:6] - - return fmt.Sprintf("%s-%s-%s", name, ns, hashSuffix) - -} - -// BuildPodLabelSelector returns a label selector to match the otterize server labels for an intents resource -func (in *ClientIntents) BuildPodLabelSelector() (labels.Selector, error) { - labelSelector, err := labels.Parse( - fmt.Sprintf("%s=%s", - OtterizeServiceLabelKey, - // Since all pods are also labeled with their server identity, we can use the Otterize server label - // To find all pods for this specific service - GetFormattedOtterizeIdentity(in.Spec.Service.Name, in.Namespace))) - if err != nil { - return nil, errors.Wrap(err) - } - - return labelSelector, nil -} - func (in *ClientIntents) HasKafkaTypeInCallList() bool { for _, intent := range in.GetCallsList() { if intent.Type == IntentTypeKafka { diff --git a/src/operator/api/v1alpha3/otterize_labels.go b/src/operator/api/v1alpha3/otterize_labels.go index 4b395ab49..74437a0f2 100644 --- a/src/operator/api/v1alpha3/otterize_labels.go +++ b/src/operator/api/v1alpha3/otterize_labels.go @@ -1,7 +1,15 @@ package v1alpha3 import ( + "context" + "fmt" + "github.com/otterize/intents-operator/src/shared/errors" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" + "golang.org/x/exp/maps" v1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "strings" ) @@ -23,15 +31,16 @@ func IsMissingOtterizeAccessLabels(pod *v1.Pod, otterizeAccessLabels map[string] // UpdateOtterizeAccessLabels updates a pod's labels with Otterize labels representing their intents // The pod is also labeled with "otterize-client=" to mark it as having intents or being the client-side of an egress netpol -func UpdateOtterizeAccessLabels(pod *v1.Pod, serviceName string, otterizeAccessLabels map[string]string) *v1.Pod { +func UpdateOtterizeAccessLabels(pod *v1.Pod, serviceIdentity serviceidentity.ServiceIdentity, otterizeAccessLabels map[string]string) *v1.Pod { if pod.Labels == nil { pod.Labels = make(map[string]string) } + pod = cleanupOtterizeLabelsAndAnnotations(pod) for k, v := range otterizeAccessLabels { pod.Labels[k] = v } - pod.Labels[OtterizeClientLabelKey] = GetFormattedOtterizeIdentity(serviceName, pod.Namespace) + pod.Labels[OtterizeClientLabelKey] = serviceIdentity.GetFormattedOtterizeIdentityWithoutKind() return pod } @@ -40,6 +49,11 @@ func HasOtterizeServiceLabel(pod *v1.Pod, labelValue string) bool { return exists && value == labelValue } +func HasOtterizeOwnerKindLabel(pod *v1.Pod, labelValue string) bool { + value, exists := pod.Labels[OtterizeOwnerKindLabelKey] + return exists && value == labelValue +} + func HasOtterizeDeprecatedServerLabel(pod *v1.Pod) bool { _, exists := pod.Labels[OtterizeServerLabelKeyDeprecated] return exists @@ -61,20 +75,6 @@ func isOtterizeAccessLabel(s string) bool { return strings.HasPrefix(s, OtterizeAccessLabelPrefix) || strings.HasPrefix(s, OtterizeServiceAccessLabelPrefix) } -func CleanupOtterizeKubernetesServiceLabels(pod *v1.Pod) *v1.Pod { - for k := range pod.Labels { - if isOtterizeKubernetesServiceLabel(k) { - delete(pod.Labels, k) - } - } - - return pod -} - -func isOtterizeKubernetesServiceLabel(s string) bool { - return strings.HasPrefix(s, OtterizeKubernetesServiceLabelKeyPrefix) -} - func GetOtterizeLabelsFromPod(pod *v1.Pod) map[string]string { otterizeLabels := make(map[string]string) for k, v := range pod.Labels { @@ -85,3 +85,29 @@ func GetOtterizeLabelsFromPod(pod *v1.Pod) map[string]string { return otterizeLabels } + +func ServiceIdentityToLabelsForWorkloadSelection(ctx context.Context, k8sClient client.Client, identity serviceidentity.ServiceIdentity) (map[string]string, bool, error) { + // This is here for backwards compatibility + if identity.Kind == "" || identity.Kind == serviceidentity.KindOtterizeLegacy { + return map[string]string{OtterizeServiceLabelKey: identity.GetFormattedOtterizeIdentityWithoutKind()}, true, nil + } + + if identity.Kind == serviceidentity.KindService { + svc := v1.Service{} + err := k8sClient.Get(ctx, types.NamespacedName{Name: identity.Name, Namespace: identity.Namespace}, &svc) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil, false, nil + } + return nil, false, errors.Wrap(err) + } + if svc.Spec.Selector == nil { + return nil, false, fmt.Errorf("service %s/%s has no selector", svc.Namespace, svc.Name) + } + return maps.Clone(svc.Spec.Selector), true, nil + } + + // This should be replaced with a logic that gets the pod owners and uses its labelsSelector (for known kinds) + return map[string]string{OtterizeOwnerKindLabelKey: identity.Kind, + OtterizeServiceLabelKey: identity.GetFormattedOtterizeIdentityWithoutKind()}, true, nil +} diff --git a/src/operator/api/v1alpha3/protectedservice_types.go b/src/operator/api/v1alpha3/protectedservice_types.go index c412fb4b4..3f5040549 100644 --- a/src/operator/api/v1alpha3/protectedservice_types.go +++ b/src/operator/api/v1alpha3/protectedservice_types.go @@ -17,12 +17,16 @@ limitations under the License. package v1alpha3 import ( + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // ProtectedServiceSpec defines the desired state of ProtectedService type ProtectedServiceSpec struct { Name string `json:"name,omitempty"` + + //+optional + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` } // ProtectedServiceStatus defines the observed state of ProtectedService @@ -44,6 +48,13 @@ type ProtectedService struct { Status ProtectedServiceStatus `json:"status,omitempty"` } +func (in *ProtectedService) GetKind() string { + if in.Spec.Kind == "" { + return serviceidentity.KindOtterizeLegacy + } + return in.Spec.Kind +} + //+kubebuilder:object:root=true // ProtectedServiceList contains a list of ProtectedService diff --git a/src/operator/api/v1alpha3/serviceidentity.go b/src/operator/api/v1alpha3/serviceidentity.go new file mode 100644 index 000000000..13b65ecf8 --- /dev/null +++ b/src/operator/api/v1alpha3/serviceidentity.go @@ -0,0 +1,27 @@ +package v1alpha3 + +import "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" + +func (in *ClientIntents) ToServiceIdentity() serviceidentity.ServiceIdentity { + return serviceidentity.ServiceIdentity{ + Name: in.Spec.Service.Name, + Namespace: in.Namespace, + Kind: in.GetClientKind(), + } +} + +func (in *Intent) ToServiceIdentity(intentsObjNamespace string) serviceidentity.ServiceIdentity { + return serviceidentity.ServiceIdentity{ + Name: in.GetTargetServerName(), + Namespace: in.GetTargetServerNamespace(intentsObjNamespace), + Kind: in.GetTargetServerKind(), + } +} + +func (in *ProtectedService) ToServiceIdentity() serviceidentity.ServiceIdentity { + return serviceidentity.ServiceIdentity{ + Name: in.Spec.Name, + Namespace: in.Namespace, + Kind: in.GetKind(), + } +} diff --git a/src/operator/config/crd/k8s.otterize.com_clientintents.patched b/src/operator/config/crd/k8s.otterize.com_clientintents.patched index edcf6395a..1a2cb0e53 100644 --- a/src/operator/config/crd/k8s.otterize.com_clientintents.patched +++ b/src/operator/config/crd/k8s.otterize.com_clientintents.patched @@ -372,6 +372,8 @@ spec: - operations type: object type: array + kind: + type: string name: type: string type: @@ -388,6 +390,8 @@ spec: type: array service: properties: + kind: + type: string name: type: string required: diff --git a/src/operator/config/crd/k8s.otterize.com_clientintents.yaml b/src/operator/config/crd/k8s.otterize.com_clientintents.yaml index 79d2d267a..48912ef8f 100644 --- a/src/operator/config/crd/k8s.otterize.com_clientintents.yaml +++ b/src/operator/config/crd/k8s.otterize.com_clientintents.yaml @@ -359,6 +359,8 @@ spec: - operations type: object type: array + kind: + type: string name: type: string type: @@ -375,6 +377,8 @@ spec: type: array service: properties: + kind: + type: string name: type: string required: diff --git a/src/operator/config/crd/k8s.otterize.com_kafkaserverconfigs.patched b/src/operator/config/crd/k8s.otterize.com_kafkaserverconfigs.patched index 935498a07..13e8d66b9 100644 --- a/src/operator/config/crd/k8s.otterize.com_kafkaserverconfigs.patched +++ b/src/operator/config/crd/k8s.otterize.com_kafkaserverconfigs.patched @@ -144,6 +144,8 @@ spec: type: boolean service: properties: + kind: + type: string name: type: string required: diff --git a/src/operator/config/crd/k8s.otterize.com_kafkaserverconfigs.yaml b/src/operator/config/crd/k8s.otterize.com_kafkaserverconfigs.yaml index 8b76aea27..7342a1147 100644 --- a/src/operator/config/crd/k8s.otterize.com_kafkaserverconfigs.yaml +++ b/src/operator/config/crd/k8s.otterize.com_kafkaserverconfigs.yaml @@ -130,6 +130,8 @@ spec: type: boolean service: properties: + kind: + type: string name: type: string required: diff --git a/src/operator/config/crd/k8s.otterize.com_protectedservices.patched b/src/operator/config/crd/k8s.otterize.com_protectedservices.patched index aef1f8ba6..856532207 100644 --- a/src/operator/config/crd/k8s.otterize.com_protectedservices.patched +++ b/src/operator/config/crd/k8s.otterize.com_protectedservices.patched @@ -89,6 +89,8 @@ spec: spec: description: ProtectedServiceSpec defines the desired state of ProtectedService properties: + kind: + type: string name: type: string type: object diff --git a/src/operator/config/crd/k8s.otterize.com_protectedservices.yaml b/src/operator/config/crd/k8s.otterize.com_protectedservices.yaml index 0cbc3537b..584fa7f91 100644 --- a/src/operator/config/crd/k8s.otterize.com_protectedservices.yaml +++ b/src/operator/config/crd/k8s.otterize.com_protectedservices.yaml @@ -75,6 +75,8 @@ spec: spec: description: ProtectedServiceSpec defines the desired state of ProtectedService properties: + kind: + type: string name: type: string type: object diff --git a/src/operator/controllers/external_traffic/network_policy.go b/src/operator/controllers/external_traffic/network_policy.go index 27e792220..7142acdfd 100644 --- a/src/operator/controllers/external_traffic/network_policy.go +++ b/src/operator/controllers/external_traffic/network_policy.go @@ -7,6 +7,8 @@ import ( "github.com/otterize/intents-operator/src/shared/errors" "github.com/otterize/intents-operator/src/shared/injectablerecorder" "github.com/otterize/intents-operator/src/shared/operatorconfig/allowexternaltraffic" + "github.com/otterize/intents-operator/src/shared/serviceidresolver" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/samber/lo" "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" @@ -344,20 +346,35 @@ func (r *NetworkPolicyHandler) handleEndpointsWithIngressList(ctx context.Contex if !ok { continue } + netpolSlice := make([]v1.NetworkPolicy, 0) + serviceId, err := serviceidresolver.NewResolver(r.client).ResolvePodToServiceIdentity(ctx, pod) + if err != nil { + return errors.Wrap(err) + } netpolList := &v1.NetworkPolicyList{} + // Get netpolSlice which was created by intents targeting this pod by its owner with "kind" + err = r.client.List(ctx, netpolList, client.MatchingLabels{v1alpha3.OtterizeNetworkPolicy: serviceId.GetFormattedOtterizeIdentityWithKind()}) + if err != nil { + return errors.Wrap(err) + } + netpolSlice = append(netpolSlice, netpolList.Items...) + // Get netpolSlice which was created by intents targeting this pod by its owner without "kind" + err = r.client.List(ctx, netpolList, client.MatchingLabels{v1alpha3.OtterizeNetworkPolicy: serviceId.GetFormattedOtterizeIdentityWithoutKind()}) + if err != nil { + return errors.Wrap(err) + } + netpolSlice = append(netpolSlice, netpolList.Items...) - err = r.client.List(ctx, netpolList, client.MatchingLabels{v1alpha3.OtterizeNetworkPolicy: serverLabel}) + // Get netpolSlice which was created by intents targeting this pod by its service + err = r.client.List(ctx, netpolList, client.MatchingLabels{v1alpha3.OtterizeNetworkPolicy: (&serviceidentity.ServiceIdentity{Name: endpoints.Name, Namespace: endpoints.Namespace, Kind: serviceidentity.KindService}).GetFormattedOtterizeIdentityWithKind()}) if err != nil { - if k8serrors.IsNotFound(err) { - // only act on pods affected by Otterize policies - if they were not created yet, - // the intents reconciler will call the endpoints reconciler once it does. - continue - } return errors.Wrap(err) } - hasIngressRules := lo.SomeBy(netpolList.Items, func(netpol v1.NetworkPolicy) bool { + netpolSlice = append(netpolSlice, netpolList.Items...) + + hasIngressRules := lo.SomeBy(netpolSlice, func(netpol v1.NetworkPolicy) bool { return lo.Contains(netpol.Spec.PolicyTypes, v1.PolicyTypeIngress) }) @@ -371,9 +388,6 @@ func (r *NetworkPolicyHandler) handleEndpointsWithIngressList(ctx context.Contex continue } - netpolSlice := make([]v1.NetworkPolicy, 0) - netpolSlice = append(netpolSlice, netpolList.Items...) - foundOtterizeNetpolsAffectingPods = true err = r.handleNetpolsForOtterizeService(ctx, endpoints, serverLabel, ingressList, netpolSlice) if err != nil { diff --git a/src/operator/controllers/intents_controller.go b/src/operator/controllers/intents_controller.go index 23cf11f17..f7b5c32f9 100644 --- a/src/operator/controllers/intents_controller.go +++ b/src/operator/controllers/intents_controller.go @@ -29,7 +29,6 @@ import ( "github.com/otterize/intents-operator/src/shared/operator_cloud_client" "github.com/otterize/intents-operator/src/shared/reconcilergroup" "github.com/otterize/intents-operator/src/shared/serviceidresolver" - "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/otterize/intents-operator/src/shared/telemetries/telemetriesconfig" "github.com/samber/lo" "github.com/sirupsen/logrus" @@ -344,8 +343,8 @@ func (r *IntentsReconciler) InitIntentsServerIndices(mgr ctrl.Manager) error { res = append(res, otterizev1alpha3.OtterizeInternetTargetName) continue } - service := serviceidentity.NewFromIntent(intent, intents.Namespace) - res = append(res, service.GetFormattedOtterizeIdentity()) + service := intent.ToServiceIdentity(intents.Namespace) + res = append(res, service.GetFormattedOtterizeIdentityWithKind()) } return res diff --git a/src/operator/controllers/intents_controller_test.go b/src/operator/controllers/intents_controller_test.go index 2a8a70743..e52a9b705 100644 --- a/src/operator/controllers/intents_controller_test.go +++ b/src/operator/controllers/intents_controller_test.go @@ -2,7 +2,6 @@ package controllers import ( "context" - otterizev1alpha2 "github.com/otterize/intents-operator/src/operator/api/v1alpha2" otterizev1alpha3 "github.com/otterize/intents-operator/src/operator/api/v1alpha3" "github.com/otterize/intents-operator/src/shared/testbase" "github.com/stretchr/testify/suite" @@ -91,7 +90,7 @@ func (s *IntentsControllerTestSuite) TestMappingProtectedServicesToIntent() { s.Client.EXPECT().List( gomock.Any(), &otterizev1alpha3.ClientIntentsList{}, - &client.MatchingFields{otterizev1alpha2.OtterizeTargetServerIndexField: fullServerName}, + &client.MatchingFields{otterizev1alpha3.OtterizeTargetServerIndexField: fullServerName}, ).DoAndReturn( func(ctx context.Context, list *otterizev1alpha3.ClientIntentsList, opts ...client.ListOption) error { list.Items = clientIntents @@ -131,7 +130,7 @@ func (s *IntentsControllerTestSuite) TestMappingProtectedServicesToIntentNoInten s.Client.EXPECT().List( gomock.Any(), &otterizev1alpha3.ClientIntentsList{}, - &client.MatchingFields{otterizev1alpha2.OtterizeTargetServerIndexField: fullServerName}, + &client.MatchingFields{otterizev1alpha3.OtterizeTargetServerIndexField: fullServerName}, ).Return(nil) expected := make([]reconcile.Request, 0) diff --git a/src/operator/controllers/intents_reconcilers/external_traffic_network_policy/external_traffic_network_policy_test.go b/src/operator/controllers/intents_reconcilers/external_traffic_network_policy/external_traffic_network_policy_test.go index ade968f65..b87bc87ca 100644 --- a/src/operator/controllers/intents_reconcilers/external_traffic_network_policy/external_traffic_network_policy_test.go +++ b/src/operator/controllers/intents_reconcilers/external_traffic_network_policy/external_traffic_network_policy_test.go @@ -16,6 +16,7 @@ import ( "github.com/otterize/intents-operator/src/operator/controllers/protected_service_reconcilers" "github.com/otterize/intents-operator/src/operator/effectivepolicy" "github.com/otterize/intents-operator/src/shared/operatorconfig/allowexternaltraffic" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/otterize/intents-operator/src/shared/testbase" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" @@ -73,7 +74,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) SetupTest() { defaultActive := !isShadowMode netpolHandler := external_traffic.NewNetworkPolicyHandler(s.Mgr.GetClient(), s.TestEnv.Scheme, allowexternaltraffic.IfBlockedByOtterize) s.defaultDenyReconciler = protected_service_reconcilers.NewDefaultDenyReconciler(s.Mgr.GetClient(), netpolHandler, true) - netpolReconciler := networkpolicy.NewReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, netpolHandler, []string{}, goset.NewSet[string](), true, defaultActive, []networkpolicy.IngressRuleBuilder{builders.NewIngressNetpolBuilder()}, nil) + netpolReconciler := networkpolicy.NewReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, netpolHandler, []string{}, goset.NewSet[string](), true, defaultActive, []networkpolicy.IngressRuleBuilder{builders.NewIngressNetpolBuilder(), builders.NewPortNetworkPolicyReconciler(s.Mgr.GetClient())}, nil) epReconciler := effectivepolicy.NewGroupReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, netpolReconciler) s.EffectivePolicyIntentsReconciler = intents_reconcilers.NewServiceEffectiveIntentsReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, epReconciler) s.Require().NoError((&controllers.IntentsReconciler{}).InitIntentsServerIndices(s.Mgr)) @@ -98,7 +99,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) SetupTest() { func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForIngress() { serviceName := "test-server-ingress-test" - intents, err := s.AddIntents("test-intents", "test-client", []otterizev1alpha3.Intent{{ + intents, err := s.AddIntents("test-intents", "test-client", "Deployment", []otterizev1alpha3.Intent{{ Type: otterizev1alpha3.IntentTypeHTTP, Name: serviceName, }, }) @@ -153,6 +154,118 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForIng }) } +func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForIngressWithIntentToSVC() { + serviceName := "test-server-ingress-test" + intents, err := s.AddIntents("test-intents", "test-client", "Deployment", []otterizev1alpha3.Intent{{ + Type: otterizev1alpha3.IntentTypeHTTP, Name: serviceName, Kind: serviceidentity.KindService, + }, + }) + s.Require().NoError(err) + s.AddDeploymentWithService(serviceName, []string{"1.1.1.1"}, map[string]string{"app": "test"}, nil) + + // the ingress reconciler expect the pod watcher labels in order to work + _, err = s.podWatcher.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: s.TestNamespace, Name: serviceName + "-0"}}) + s.Require().NoError(err) + + res, err := s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: s.TestNamespace, + Name: intents.Name, + }, + }) + s.Require().NoError(err) + s.Require().Empty(res) + + // make sure the network policy was created between the two services based on the intents + np := &v1.NetworkPolicy{} + policyName := fmt.Sprintf(otterizev1alpha3.OtterizeSingleNetworkPolicyNameTemplate, fmt.Sprintf("%s-service", serviceName)) + s.WaitUntilCondition(func(assert *assert.Assertions) { + err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: policyName}, np) + assert.NoError(err) + assert.NotEmpty(np) + }) + + // make sure the ingress network policy doesn't exist yet + externalNetworkPolicyName := fmt.Sprintf(external_traffic.OtterizeExternalNetworkPolicyNameTemplate, serviceName) + s.WaitUntilCondition(func(assert *assert.Assertions) { + err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np) + assert.True(errors.IsNotFound(err)) + }) + s.AddIngress(serviceName) + + res, err = s.IngressReconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: s.TestNamespace, + Name: serviceName + "-ingress", + }, + }) + + s.Require().NoError(err) + s.Require().Empty(res) + + s.WaitUntilCondition(func(assert *assert.Assertions) { + err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np) + assert.NoError(err) + assert.NotEmpty(np) + }) +} + +func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForIngressWithIntentToDeployment() { + serviceName := "test-server-ingress-test" + intents, err := s.AddIntents("test-intents", "test-client", "Deployment", []otterizev1alpha3.Intent{{ + Type: otterizev1alpha3.IntentTypeHTTP, Name: serviceName, Kind: "Deployment", + }, + }) + s.Require().NoError(err) + s.AddDeploymentWithService(serviceName, []string{"1.1.1.1"}, map[string]string{"app": "test"}, nil) + + // the ingress reconciler expect the pod watcher labels in order to work + _, err = s.podWatcher.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: s.TestNamespace, Name: serviceName + "-0"}}) + s.Require().NoError(err) + + res, err := s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: s.TestNamespace, + Name: intents.Name, + }, + }) + s.Require().NoError(err) + s.Require().Empty(res) + + // make sure the network policy was created between the two services based on the intents + np := &v1.NetworkPolicy{} + policyName := fmt.Sprintf(otterizev1alpha3.OtterizeSingleNetworkPolicyNameTemplate, fmt.Sprintf("%s-deployment", serviceName)) + s.WaitUntilCondition(func(assert *assert.Assertions) { + err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: policyName}, np) + assert.NoError(err) + assert.NotEmpty(np) + }) + + // make sure the ingress network policy doesn't exist yet + externalNetworkPolicyName := fmt.Sprintf(external_traffic.OtterizeExternalNetworkPolicyNameTemplate, serviceName) + s.WaitUntilCondition(func(assert *assert.Assertions) { + err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np) + assert.True(errors.IsNotFound(err)) + }) + s.AddIngress(serviceName) + + res, err = s.IngressReconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: s.TestNamespace, + Name: serviceName + "-ingress", + }, + }) + + s.Require().NoError(err) + s.Require().Empty(res) + + s.WaitUntilCondition(func(assert *assert.Assertions) { + err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np) + assert.NoError(err) + assert.NotEmpty(np) + }) +} + func (s *ExternalNetworkPolicyReconcilerTestSuite) TestIngressProtectedService_ShadowMode() { serviceName := "test-server-ingress-test" @@ -262,7 +375,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestIngressWithIntentsProtect assert.NotEmpty(defaultDenyPolicy) }) - _, err = s.AddIntents("test-intents", "test-client", []otterizev1alpha3.Intent{{ + _, err = s.AddIntents("test-intents", "test-client", "Deployment", []otterizev1alpha3.Intent{{ Type: otterizev1alpha3.IntentTypeHTTP, Name: serviceName, }, }) @@ -312,7 +425,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestIngressWithIntentsProtect func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoadBalancer() { serviceName := "test-server-load-balancer-test" - intents, err := s.AddIntents("test-intents", "test-client", []otterizev1alpha3.Intent{{ + intents, err := s.AddIntents("test-intents", "test-client", "Deployment", []otterizev1alpha3.Intent{{ Type: otterizev1alpha3.IntentTypeHTTP, Name: serviceName, }, }) @@ -373,7 +486,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoadBalancerCreatedAndDeletedWhenLastIntentDeleted() { serviceName := "test-server-load-balancer-test" - intents, err := s.AddIntents("test-intents", "test-client", []otterizev1alpha3.Intent{{ + intents, err := s.AddIntents("test-intents", "test-client", "Deployment", []otterizev1alpha3.Intent{{ Name: serviceName, }, }) @@ -468,7 +581,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoadBalancerCreatedAndDoesNotGetDeletedEvenWhenIntentRemovedAsLongAsOneRemains() { serviceName := "test-server-load-balancer-test" - intents, err := s.AddIntents("test-intents", "test-client", []otterizev1alpha3.Intent{{ + intents, err := s.AddIntents("test-intents", "test-client", "Deployment", []otterizev1alpha3.Intent{{ Name: serviceName, }, }) @@ -476,7 +589,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa secondaryNamespace := "ns-" + uuid.New().String() + "e" s.CreateNamespace(secondaryNamespace) - secondIntents, err := s.AddIntentsInNamespace("test-intents-other-ns", "test-client-other-ns", secondaryNamespace, []otterizev1alpha3.Intent{{ + secondIntents, err := s.AddIntentsInNamespace("test-intents-other-ns", "test-client-other-ns", "", secondaryNamespace, []otterizev1alpha3.Intent{{ Name: fmt.Sprintf("%s.%s", serviceName, s.TestNamespace), }}) s.Require().NoError(err) @@ -578,7 +691,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForNodePort() { serviceName := "test-server-node-port-test" - intents, err := s.AddIntents("test-intents", "test-client", []otterizev1alpha3.Intent{{ + intents, err := s.AddIntents("test-intents", "test-client", "Deployment", []otterizev1alpha3.Intent{{ Name: serviceName, }, }) @@ -639,7 +752,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForNod func (s *ExternalNetworkPolicyReconcilerTestSuite) TestEndpointsReconcilerNetworkPoliciesDisabled() { serviceName := "test-endpoints-reconciler-enforcement-disabled" - intents, err := s.AddIntents("test-intents", "test-client", []otterizev1alpha3.Intent{{ + intents, err := s.AddIntents("test-intents", "test-client", "Deployment", []otterizev1alpha3.Intent{{ Name: serviceName, }, }) diff --git a/src/operator/controllers/intents_reconcilers/external_traffic_network_policy/external_traffic_network_policy_with_no_intents_test.go b/src/operator/controllers/intents_reconcilers/external_traffic_network_policy/external_traffic_network_policy_with_no_intents_test.go index 575cbb5c4..cc55b6dae 100644 --- a/src/operator/controllers/intents_reconcilers/external_traffic_network_policy/external_traffic_network_policy_with_no_intents_test.go +++ b/src/operator/controllers/intents_reconcilers/external_traffic_network_policy/external_traffic_network_policy_with_no_intents_test.go @@ -242,7 +242,7 @@ func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) TestEndpointsRec func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) TestNetworkPolicyCreateForLoadBalancerCreatedDespiteLastIntentDeleted() { serviceName := "test-server-load-balancer-test" - intents, err := s.AddIntents("test-intents", "test-client", []otterizev1alpha3.Intent{{ + intents, err := s.AddIntents("test-intents", "test-client", "Deployment", []otterizev1alpha3.Intent{{ Name: serviceName, }, }) diff --git a/src/operator/controllers/intents_reconcilers/istio_policy.go b/src/operator/controllers/intents_reconcilers/istio_policy.go index 70998c7d4..ac2971004 100644 --- a/src/operator/controllers/intents_reconcilers/istio_policy.go +++ b/src/operator/controllers/intents_reconcilers/istio_policy.go @@ -107,9 +107,14 @@ func (r *IstioPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, errors.Wrap(err) } - clientServiceAccountName := pod.Spec.ServiceAccountName missingSideCar := !istiopolicy.IsPodPartOfIstioMesh(pod) + if missingSideCar { + r.RecordWarningEvent(intents, istiopolicy.ReasonMissingSidecar, "Client pod missing sidecar, will not create policies") + logrus.Debugf("Pod %s/%s does not have a sidecar, skipping Istio policy creation", pod.Namespace, pod.Name) + return ctrl.Result{}, nil + } + clientServiceAccountName := pod.Spec.ServiceAccountName err = r.policyManager.UpdateIntentsStatus(ctx, intents, clientServiceAccountName, missingSideCar) if err != nil { return ctrl.Result{}, errors.Wrap(err) @@ -120,12 +125,6 @@ func (r *IstioPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, errors.Wrap(err) } - if missingSideCar { - r.RecordWarningEvent(intents, istiopolicy.ReasonMissingSidecar, "Client pod missing sidecar, will not create policies") - logrus.Debugf("Pod %s/%s does not have a sidecar, skipping Istio policy creation", pod.Namespace, pod.Name) - return ctrl.Result{}, nil - } - err = r.policyManager.Create(ctx, intents, clientServiceAccountName) if err != nil { if k8serrors.IsConflict(err) { @@ -134,27 +133,35 @@ func (r *IstioPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, errors.Wrap(err) } + err = r.policyManager.RemoveDeprecatedPoliciesForClient(ctx, intents) + if err != nil { + logrus.WithError(err).Debugf("Failed to remove deprecated policies for client %s", intents.Spec.Service.Name) + return ctrl.Result{}, errors.Wrap(err) + } + return ctrl.Result{}, nil } func (r *IstioPolicyReconciler) updateServerSidecarStatus(ctx context.Context, intents *otterizev1alpha3.ClientIntents) error { for _, intent := range intents.Spec.Calls { - serverNamespace := intent.GetTargetServerNamespace(intents.Namespace) - pod, err := r.serviceIdResolver.ResolveIntentServerToPod(ctx, intent, serverNamespace) + serviceId := intent.ToServiceIdentity(intents.Namespace) + pods, ok, err := r.serviceIdResolver.ResolveServiceIdentityToPodSlice(ctx, serviceId) if err != nil { - if errors.Is(err, serviceidresolver.ErrPodNotFound) { - continue - } return errors.Wrap(err) } + if !ok { + continue + } + missingSideCar := false + for _, pod := range pods { + missingSideCar = missingSideCar || !istiopolicy.IsPodPartOfIstioMesh(pod) + } - missingSideCar := !istiopolicy.IsPodPartOfIstioMesh(pod) - formattedTargetServer := otterizev1alpha3.GetFormattedOtterizeIdentity(intent.GetTargetServerName(), serverNamespace) - err = r.policyManager.UpdateServerSidecar(ctx, intents, formattedTargetServer, missingSideCar) + err = r.policyManager.UpdateServerSidecar(ctx, intents, serviceId.GetFormattedOtterizeIdentityWithKind(), missingSideCar) if err != nil { return errors.Wrap(err) } - } + } return nil } diff --git a/src/operator/controllers/intents_reconcilers/istio_policy_test.go b/src/operator/controllers/intents_reconcilers/istio_policy_test.go index 63b5eb2d4..883ca2255 100644 --- a/src/operator/controllers/intents_reconcilers/istio_policy_test.go +++ b/src/operator/controllers/intents_reconcilers/istio_policy_test.go @@ -5,6 +5,7 @@ import ( "fmt" otterizev1alpha3 "github.com/otterize/intents-operator/src/operator/api/v1alpha3" mocks "github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/mocks" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/otterize/intents-operator/src/shared/testbase" "github.com/stretchr/testify/suite" "go.uber.org/mock/gomock" @@ -23,14 +24,14 @@ import ( type IstioPolicyReconcilerTestSuite struct { testbase.MocksSuiteBase Reconciler *IstioPolicyReconciler - policyAdmin *mocks.MockAdmin + policyAdmin *mocks.MockPolicyManager serviceResolver *mocks.MockServiceResolver scheme *runtime.Scheme } func (s *IstioPolicyReconcilerTestSuite) SetupTest() { s.MocksSuiteBase.SetupTest() - s.policyAdmin = mocks.NewMockAdmin(s.Controller) + s.policyAdmin = mocks.NewMockPolicyManager(s.Controller) s.serviceResolver = mocks.NewMockServiceResolver(s.Controller) restrictToNamespaces := make([]string, 0) s.scheme = runtime.NewScheme() @@ -68,12 +69,12 @@ func (s *IstioPolicyReconcilerTestSuite) TestCreatePolicy() { NamespacedName: namespacedName, } - serverName := fmt.Sprintf("test-server.%s", serverNamespace) + serverName := "test-server" intentsSpec := &otterizev1alpha3.IntentsSpec{ Service: otterizev1alpha3.Service{Name: serviceName}, Calls: []otterizev1alpha3.Intent{ { - Name: serverName, + Name: fmt.Sprintf("%s.%s", serverName, serverNamespace), }, }, } @@ -136,10 +137,105 @@ func (s *IstioPolicyReconcilerTestSuite) TestCreatePolicy() { }, } s.serviceResolver.EXPECT().ResolveClientIntentToPod(gomock.Any(), gomock.Eq(intentsObj)).Return(clientPod, nil) + s.serviceResolver.EXPECT().ResolveServiceIdentityToPodSlice(gomock.Any(), gomock.Eq((intentsObj.Spec.Calls[0]).ToServiceIdentity(serverNamespace))).Return([]v1.Pod{serverPod}, true, nil) s.policyAdmin.EXPECT().UpdateIntentsStatus(gomock.Any(), gomock.Eq(&intentsObj), clientServiceAccount, false).Return(nil) - s.serviceResolver.EXPECT().ResolveIntentServerToPod(gomock.Any(), gomock.Eq(intentsObj.Spec.Calls[0]), serverNamespace).Return(serverPod, nil) s.policyAdmin.EXPECT().UpdateServerSidecar(gomock.Any(), gomock.Eq(&intentsObj), "test-server-far-far-away-aa0d79", false).Return(nil) s.policyAdmin.EXPECT().Create(gomock.Any(), gomock.Eq(&intentsObj), clientServiceAccount).Return(nil) + s.policyAdmin.EXPECT().RemoveDeprecatedPoliciesForClient(gomock.Any(), gomock.Eq(&intentsObj)).Return(nil) + res, err := s.Reconciler.Reconcile(context.Background(), req) + s.NoError(err) + s.Empty(res) +} + +func (s *IstioPolicyReconcilerTestSuite) TestCreatePolicyToSVC() { + clientIntentsName := "client-intents" + serviceName := "test-client" + serverNamespace := "far-far-away" + + namespacedName := types.NamespacedName{ + Namespace: testNamespace, + Name: clientIntentsName, + } + req := ctrl.Request{ + NamespacedName: namespacedName, + } + + serverName := "test-server" + intentsSpec := &otterizev1alpha3.IntentsSpec{ + Service: otterizev1alpha3.Service{Name: serviceName}, + Calls: []otterizev1alpha3.Intent{ + { + Name: fmt.Sprintf("%s.%s", serverName, serverNamespace), + Kind: serviceidentity.KindService, + }, + }, + } + + intentsWithoutFinalizer := otterizev1alpha3.ClientIntents{ + ObjectMeta: metav1.ObjectMeta{ + Name: clientIntentsName, + Namespace: testNamespace, + }, + Spec: intentsSpec, + } + + s.expectValidatingIstioIsInstalled() + + // Initial call to get the ClientIntents object when reconciler starts + emptyIntents := &otterizev1alpha3.ClientIntents{} + s.Client.EXPECT().Get(gomock.Any(), req.NamespacedName, gomock.Eq(emptyIntents)).DoAndReturn( + func(ctx context.Context, name types.NamespacedName, intents *otterizev1alpha3.ClientIntents, options ...client.ListOption) error { + intentsWithoutFinalizer.DeepCopyInto(intents) + return nil + }) + + intentsObj := otterizev1alpha3.ClientIntents{} + intentsWithoutFinalizer.DeepCopyInto(&intentsObj) + + clientServiceAccount := "test-server-sa" + clientPod := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-client-fdae32", + Namespace: serverNamespace, + }, + Spec: v1.PodSpec{ + ServiceAccountName: clientServiceAccount, + Containers: []v1.Container{ + { + Name: "real-application-who-does-something", + }, + { + Name: "istio-proxy", + }, + }, + }, + } + + serverPod := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-server-2b5e0d", + Namespace: serverNamespace, + Labels: map[string]string{"app": "test-server"}, + }, + Spec: v1.PodSpec{ + ServiceAccountName: "test-server-sa", + Containers: []v1.Container{ + { + Name: "server-who-listens", + }, + { + Name: "istio-proxy", + }, + }, + }, + } + + s.serviceResolver.EXPECT().ResolveClientIntentToPod(gomock.Any(), gomock.Eq(intentsObj)).Return(clientPod, nil) + s.serviceResolver.EXPECT().ResolveServiceIdentityToPodSlice(gomock.Any(), gomock.Eq((intentsObj.Spec.Calls[0]).ToServiceIdentity(serverNamespace))).Return([]v1.Pod{serverPod}, true, nil) + s.policyAdmin.EXPECT().UpdateIntentsStatus(gomock.Any(), gomock.Eq(&intentsObj), clientServiceAccount, false).Return(nil) + s.policyAdmin.EXPECT().UpdateServerSidecar(gomock.Any(), gomock.Eq(&intentsObj), "test-server-far-far-away-servi-558c21", false).Return(nil) + s.policyAdmin.EXPECT().Create(gomock.Any(), gomock.Eq(&intentsObj), clientServiceAccount).Return(nil) + s.policyAdmin.EXPECT().RemoveDeprecatedPoliciesForClient(gomock.Any(), gomock.Eq(&intentsObj)).Return(nil) res, err := s.Reconciler.Reconcile(context.Background(), req) s.NoError(err) s.Empty(res) @@ -239,11 +335,11 @@ func (s *IstioPolicyReconcilerTestSuite) assertPolicyCreateCalledEvenIfDisabledE } s.serviceResolver.EXPECT().ResolveClientIntentToPod(gomock.Any(), gomock.Eq(clientIntentsObj)).Return(clientPod, nil) + s.serviceResolver.EXPECT().ResolveServiceIdentityToPodSlice(gomock.Any(), gomock.Eq((clientIntentsObj.Spec.Calls[0]).ToServiceIdentity(serverNamespace))).Return([]v1.Pod{serverPod}, true, nil) s.policyAdmin.EXPECT().UpdateIntentsStatus(gomock.Any(), gomock.Eq(&clientIntentsObj), clientServiceAccount, false).Return(nil) - s.serviceResolver.EXPECT().ResolveIntentServerToPod(gomock.Any(), gomock.Eq(clientIntentsObj.Spec.Calls[0]), serverNamespace).Return(serverPod, nil) s.policyAdmin.EXPECT().UpdateServerSidecar(gomock.Any(), gomock.Eq(&clientIntentsObj), "test-server-far-far-away-aa0d79", false).Return(nil) s.policyAdmin.EXPECT().Create(gomock.Any(), gomock.Eq(&clientIntentsObj), clientServiceAccount).Return(nil) - + s.policyAdmin.EXPECT().RemoveDeprecatedPoliciesForClient(gomock.Any(), gomock.Eq(&clientIntentsObj)).Return(nil) res, err := s.Reconciler.Reconcile(context.Background(), req) s.NoError(err) s.Empty(res) @@ -293,7 +389,6 @@ func (s *IstioPolicyReconcilerTestSuite) TestIstioPolicyDeleted() { }) s.policyAdmin.EXPECT().DeleteAll(gomock.Any(), gomock.Eq(&clientIntentsObj)).Return(nil) - res, err := s.Reconciler.Reconcile(context.Background(), req) s.NoError(err) s.Empty(res) diff --git a/src/operator/controllers/intents_reconcilers/kafka_acls.go b/src/operator/controllers/intents_reconcilers/kafka_acls.go index a1f2f287b..7b222d8c7 100644 --- a/src/operator/controllers/intents_reconcilers/kafka_acls.go +++ b/src/operator/controllers/intents_reconcilers/kafka_acls.go @@ -10,6 +10,7 @@ import ( "github.com/otterize/intents-operator/src/shared/errors" "github.com/otterize/intents-operator/src/shared/injectablerecorder" "github.com/otterize/intents-operator/src/shared/serviceidresolver" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -93,7 +94,7 @@ func (r *KafkaACLReconciler) applyACLs(ctx context.Context, intents *otterizev1a if err := r.KafkaServersStore.MapErr(func(serverName types.NamespacedName, config *otterizev1alpha3.KafkaServerConfig, tls otterizev1alpha3.TLSSource) error { intentsForServer := intentsByServer[serverName] - shouldCreatePolicy, err := protected_services.IsServerEnforcementEnabledDueToProtectionOrDefaultState(ctx, r.client, serverName.Name, serverName.Namespace, r.enforcementDefaultState, r.activeNamespaces) + shouldCreatePolicy, err := protected_services.IsServerEnforcementEnabledDueToProtectionOrDefaultState(ctx, r.client, serviceidentity.ServiceIdentity{Name: serverName.Name, Namespace: serverName.Namespace}, r.enforcementDefaultState, r.activeNamespaces) if err != nil { return errors.Wrap(err) } @@ -135,7 +136,7 @@ func (r *KafkaACLReconciler) applyACLs(ctx context.Context, intents *otterizev1a func (r *KafkaACLReconciler) RemoveACLs(ctx context.Context, intents *otterizev1alpha3.ClientIntents) error { return r.KafkaServersStore.MapErr(func(serverName types.NamespacedName, config *otterizev1alpha3.KafkaServerConfig, tls otterizev1alpha3.TLSSource) error { - shouldCreatePolicy, err := protected_services.IsServerEnforcementEnabledDueToProtectionOrDefaultState(ctx, r.client, serverName.Name, serverName.Namespace, r.enforcementDefaultState, r.activeNamespaces) + shouldCreatePolicy, err := protected_services.IsServerEnforcementEnabledDueToProtectionOrDefaultState(ctx, r.client, serviceidentity.ServiceIdentity{Name: serverName.Name, Namespace: serverName.Namespace}, r.enforcementDefaultState, r.activeNamespaces) if err != nil { return errors.Wrap(err) } diff --git a/src/operator/controllers/intents_reconcilers/kafka_acls_test.go b/src/operator/controllers/intents_reconcilers/kafka_acls_test.go index 219ec9dc5..eb0b9687a 100644 --- a/src/operator/controllers/intents_reconcilers/kafka_acls_test.go +++ b/src/operator/controllers/intents_reconcilers/kafka_acls_test.go @@ -157,7 +157,7 @@ func (s *KafkaACLReconcilerTestSuite) TestNoACLCreatedForIntentsOperator() { }}, }} - _, err := s.AddIntentsInNamespace(intentsName, operatorServiceName, s.operatorNamespace, operatorIntents) + _, err := s.AddIntentsInNamespace(intentsName, operatorServiceName, "", s.operatorNamespace, operatorIntents) s.Require().NoError(err) operatorPod := corev1.Pod{ @@ -250,7 +250,7 @@ func (s *KafkaACLReconcilerTestSuite) TestKafkaACLGetCreatedAndUpdatedBasedOnInt intentsConfig := s.generateIntents(otterizev1alpha3.KafkaOperationProduce) intentsConsume := []otterizev1alpha3.Intent{intentsConfig} - _, err := s.AddIntents(intentsObjectName, clientName, intentsConsume) + _, err := s.AddIntents(intentsObjectName, clientName, "", intentsConsume) s.Require().NoError(err) namespacedName := types.NamespacedName{ Namespace: s.TestNamespace, @@ -322,7 +322,7 @@ func (s *KafkaACLReconcilerTestSuite) TestKafkaACLDeletedAfterIntentsRemoved() { intentsConfig := s.generateIntents(otterizev1alpha3.KafkaOperationConsume) intents := []otterizev1alpha3.Intent{intentsConfig} - clientIntents, err := s.AddIntents(intentsObjectName, clientName, intents) + clientIntents, err := s.AddIntents(intentsObjectName, clientName, "", intents) s.Require().NoError(err) namespacedName := types.NamespacedName{ @@ -371,7 +371,7 @@ func (s *KafkaACLReconcilerTestSuite) TestKafkaACLCreationDisabled() { intentsConfig := s.generateIntents(otterizev1alpha3.KafkaOperationConsume) intents := []otterizev1alpha3.Intent{intentsConfig} - clientIntents, err := s.AddIntents(intentsObjectName, clientName, intents) + clientIntents, err := s.AddIntents(intentsObjectName, clientName, "", intents) s.Require().NoError(err) namespacedName := types.NamespacedName{ @@ -394,7 +394,7 @@ func (s *KafkaACLReconcilerTestSuite) TestKafkaACLEnforcementGloballyDisabled() intentsConfig := s.generateIntents(otterizev1alpha3.KafkaOperationConsume) intents := []otterizev1alpha3.Intent{intentsConfig} - clientIntents, err := s.AddIntents(intentsObjectName, clientName, intents) + clientIntents, err := s.AddIntents(intentsObjectName, clientName, "", intents) s.Require().NoError(err) namespacedName := types.NamespacedName{ diff --git a/src/operator/controllers/intents_reconcilers/mocks/mock_istio_admin.go b/src/operator/controllers/intents_reconcilers/mocks/mock_istio_admin.go deleted file mode 100644 index 8f8607850..000000000 --- a/src/operator/controllers/intents_reconcilers/mocks/mock_istio_admin.go +++ /dev/null @@ -1,92 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: ../istiopolicy/admin.go - -// Package intentsreconcilersmocks is a generated GoMock package. -package intentsreconcilersmocks - -import ( - context "context" - "github.com/otterize/intents-operator/src/operator/api/v1alpha3" - reflect "reflect" - - gomock "go.uber.org/mock/gomock" -) - -// MockAdmin is a mock of Admin interface. -type MockAdmin struct { - ctrl *gomock.Controller - recorder *MockAdminMockRecorder -} - -// MockAdminMockRecorder is the mock recorder for MockAdmin. -type MockAdminMockRecorder struct { - mock *MockAdmin -} - -// NewMockAdmin creates a new mock instance. -func NewMockAdmin(ctrl *gomock.Controller) *MockAdmin { - mock := &MockAdmin{ctrl: ctrl} - mock.recorder = &MockAdminMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockAdmin) EXPECT() *MockAdminMockRecorder { - return m.recorder -} - -// Create mocks base method. -func (m *MockAdmin) Create(ctx context.Context, clientIntents *v1alpha3.ClientIntents, clientServiceAccount string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Create", ctx, clientIntents, clientServiceAccount) - ret0, _ := ret[0].(error) - return ret0 -} - -// Create indicates an expected call of Create. -func (mr *MockAdminMockRecorder) Create(ctx, clientIntents, clientServiceAccount interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Create", reflect.TypeOf((*MockAdmin)(nil).Create), ctx, clientIntents, clientServiceAccount) -} - -// DeleteAll mocks base method. -func (m *MockAdmin) DeleteAll(ctx context.Context, clientIntents *v1alpha3.ClientIntents) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAll", ctx, clientIntents) - ret0, _ := ret[0].(error) - return ret0 -} - -// DeleteAll indicates an expected call of DeleteAll. -func (mr *MockAdminMockRecorder) DeleteAll(ctx, clientIntents interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAll", reflect.TypeOf((*MockAdmin)(nil).DeleteAll), ctx, clientIntents) -} - -// UpdateIntentsStatus mocks base method. -func (m *MockAdmin) UpdateIntentsStatus(ctx context.Context, clientIntents *v1alpha3.ClientIntents, clientServiceAccount string, missingSideCar bool) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateIntentsStatus", ctx, clientIntents, clientServiceAccount, missingSideCar) - ret0, _ := ret[0].(error) - return ret0 -} - -// UpdateIntentsStatus indicates an expected call of UpdateIntentsStatus. -func (mr *MockAdminMockRecorder) UpdateIntentsStatus(ctx, clientIntents, clientServiceAccount, missingSideCar interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateIntentsStatus", reflect.TypeOf((*MockAdmin)(nil).UpdateIntentsStatus), ctx, clientIntents, clientServiceAccount, missingSideCar) -} - -// UpdateServerSidecar mocks base method. -func (m *MockAdmin) UpdateServerSidecar(ctx context.Context, clientIntents *v1alpha3.ClientIntents, serverName string, missingSideCar bool) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateServerSidecar", ctx, clientIntents, serverName, missingSideCar) - ret0, _ := ret[0].(error) - return ret0 -} - -// UpdateServerSidecar indicates an expected call of UpdateServerSidecar. -func (mr *MockAdminMockRecorder) UpdateServerSidecar(ctx, clientIntents, serverName, missingSideCar interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateServerSidecar", reflect.TypeOf((*MockAdmin)(nil).UpdateServerSidecar), ctx, clientIntents, serverName, missingSideCar) -} diff --git a/src/operator/controllers/intents_reconcilers/mocks/mock_istio_manager.go b/src/operator/controllers/intents_reconcilers/mocks/mock_istio_manager.go index c9226640e..769b74c58 100644 --- a/src/operator/controllers/intents_reconcilers/mocks/mock_istio_manager.go +++ b/src/operator/controllers/intents_reconcilers/mocks/mock_istio_manager.go @@ -63,6 +63,20 @@ func (mr *MockPolicyManagerMockRecorder) DeleteAll(ctx, clientIntents interface{ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAll", reflect.TypeOf((*MockPolicyManager)(nil).DeleteAll), ctx, clientIntents) } +// RemoveDeprecatedPoliciesForClient mocks base method. +func (m *MockPolicyManager) RemoveDeprecatedPoliciesForClient(ctx context.Context, clientIntents *v1alpha3.ClientIntents) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveDeprecatedPoliciesForClient", ctx, clientIntents) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveDeprecatedPoliciesForClient indicates an expected call of RemoveDeprecatedPoliciesForClient. +func (mr *MockPolicyManagerMockRecorder) RemoveDeprecatedPoliciesForClient(ctx, clientIntents interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveDeprecatedPoliciesForClient", reflect.TypeOf((*MockPolicyManager)(nil).RemoveDeprecatedPoliciesForClient), ctx, clientIntents) +} + // UpdateIntentsStatus mocks base method. func (m *MockPolicyManager) UpdateIntentsStatus(ctx context.Context, clientIntents *v1alpha3.ClientIntents, clientServiceAccount string, missingSideCar bool) error { m.ctrl.T.Helper() diff --git a/src/operator/controllers/intents_reconcilers/mocks/mock_service_resolver.go b/src/operator/controllers/intents_reconcilers/mocks/mock_service_resolver.go index f55db8b9a..82a0279ea 100644 --- a/src/operator/controllers/intents_reconcilers/mocks/mock_service_resolver.go +++ b/src/operator/controllers/intents_reconcilers/mocks/mock_service_resolver.go @@ -37,21 +37,6 @@ func (m *MockServiceResolver) EXPECT() *MockServiceResolverMockRecorder { return m.recorder } -// GetKubernetesServicesTargetingPod mocks base method. -func (m *MockServiceResolver) GetKubernetesServicesTargetingPod(ctx context.Context, pod *v1.Pod) ([]v1.Service, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetKubernetesServicesTargetingPod", ctx, pod) - ret0, _ := ret[0].([]v1.Service) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetKubernetesServicesTargetingPod indicates an expected call of GetKubernetesServicesTargetingPod. -func (mr *MockServiceResolverMockRecorder) GetKubernetesServicesTargetingPod(ctx, pod interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKubernetesServicesTargetingPod", reflect.TypeOf((*MockServiceResolver)(nil).GetKubernetesServicesTargetingPod), ctx, pod) -} - // ResolveClientIntentToPod mocks base method. func (m *MockServiceResolver) ResolveClientIntentToPod(ctx context.Context, intent v1alpha3.ClientIntents) (v1.Pod, error) { m.ctrl.T.Helper() @@ -67,21 +52,6 @@ func (mr *MockServiceResolverMockRecorder) ResolveClientIntentToPod(ctx, intent return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveClientIntentToPod", reflect.TypeOf((*MockServiceResolver)(nil).ResolveClientIntentToPod), ctx, intent) } -// ResolveIntentServerToPod mocks base method. -func (m *MockServiceResolver) ResolveIntentServerToPod(ctx context.Context, intent v1alpha3.Intent, namespace string) (v1.Pod, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ResolveIntentServerToPod", ctx, intent, namespace) - ret0, _ := ret[0].(v1.Pod) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ResolveIntentServerToPod indicates an expected call of ResolveIntentServerToPod. -func (mr *MockServiceResolverMockRecorder) ResolveIntentServerToPod(ctx, intent, namespace interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveIntentServerToPod", reflect.TypeOf((*MockServiceResolver)(nil).ResolveIntentServerToPod), ctx, intent, namespace) -} - // ResolvePodToServiceIdentity mocks base method. func (m *MockServiceResolver) ResolvePodToServiceIdentity(ctx context.Context, pod *v1.Pod) (serviceidentity.ServiceIdentity, error) { m.ctrl.T.Helper() @@ -96,3 +66,19 @@ func (mr *MockServiceResolverMockRecorder) ResolvePodToServiceIdentity(ctx, pod mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolvePodToServiceIdentity", reflect.TypeOf((*MockServiceResolver)(nil).ResolvePodToServiceIdentity), ctx, pod) } + +// ResolveServiceIdentityToPodSlice mocks base method. +func (m *MockServiceResolver) ResolveServiceIdentityToPodSlice(ctx context.Context, identity serviceidentity.ServiceIdentity) ([]v1.Pod, bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ResolveServiceIdentityToPodSlice", ctx, identity) + ret0, _ := ret[0].([]v1.Pod) + ret1, _ := ret[1].(bool) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// ResolveServiceIdentityToPodSlice indicates an expected call of ResolveServiceIdentityToPodSlice. +func (mr *MockServiceResolverMockRecorder) ResolveServiceIdentityToPodSlice(ctx, identity interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveServiceIdentityToPodSlice", reflect.TypeOf((*MockServiceResolver)(nil).ResolveServiceIdentityToPodSlice), ctx, identity) +} diff --git a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/builders_test.go b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/builders_test.go index 8fb7804c2..7eb5b220c 100644 --- a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/builders_test.go +++ b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/builders_test.go @@ -5,6 +5,7 @@ import ( "fmt" otterizev1alpha3 "github.com/otterize/intents-operator/src/operator/api/v1alpha3" "github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/consts" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/stretchr/testify/suite" "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" @@ -36,9 +37,9 @@ func (s *AllBuildersTestSuite) TestCreateEveryRuleKind() { serviceName := "test-client" serverName := "test-server" serverNamespace := testServerNamespace - formattedServer := otterizev1alpha3.GetFormattedOtterizeIdentity(serverName, serverNamespace) - formattedServerService := otterizev1alpha3.GetFormattedOtterizeIdentity("svc."+serverName, serverNamespace) - formattedClient := otterizev1alpha3.GetFormattedOtterizeIdentity(serviceName, testNamespace) + formattedServer := (&serviceidentity.ServiceIdentity{Name: serverName, Namespace: serverNamespace}).GetFormattedOtterizeIdentityWithoutKind() + formattedServerService := (&serviceidentity.ServiceIdentity{Name: serverName, Namespace: serverNamespace, Kind: serviceidentity.KindService}).GetFormattedOtterizeIdentityWithKind() + formattedClient := (&serviceidentity.ServiceIdentity{Name: serviceName, Namespace: testNamespace}).GetFormattedOtterizeIdentityWithoutKind() namespacedName := types.NamespacedName{ Namespace: testNamespace, Name: "client-intents", @@ -181,6 +182,164 @@ func (s *AllBuildersTestSuite) TestCreateEveryRuleKind() { s.Empty(res) } +func (s *AllBuildersTestSuite) TestCreateEveryRuleKindWithKinds() { + serviceName := "test-client" + serverName := "test-server" + serverNamespace := testServerNamespace + formattedServerWithoutKind := (&serviceidentity.ServiceIdentity{Name: serverName, Namespace: serverNamespace, Kind: "Deployment"}).GetFormattedOtterizeIdentityWithoutKind() + formattedServerWithKind := (&serviceidentity.ServiceIdentity{Name: serverName, Namespace: serverNamespace, Kind: "Deployment"}).GetFormattedOtterizeIdentityWithKind() + formattedServerService := (&serviceidentity.ServiceIdentity{Name: serverName, Namespace: serverNamespace, Kind: serviceidentity.KindService}).GetFormattedOtterizeIdentityWithKind() + formattedClientWithoutKind := (&serviceidentity.ServiceIdentity{Name: serviceName, Namespace: testNamespace, Kind: "Deployment"}).GetFormattedOtterizeIdentityWithoutKind() + formattedClientWithKind := (&serviceidentity.ServiceIdentity{Name: serviceName, Namespace: testNamespace, Kind: "Deployment"}).GetFormattedOtterizeIdentityWithKind() + namespacedName := types.NamespacedName{ + Namespace: testNamespace, + Name: "client-intents", + } + req := ctrl.Request{ + NamespacedName: namespacedName, + } + intentsSpec := &otterizev1alpha3.IntentsSpec{ + Service: otterizev1alpha3.Service{Name: serviceName, Kind: "Deployment"}, + Calls: []otterizev1alpha3.Intent{ + { + Name: fmt.Sprintf("%s.%s", serverName, serverNamespace), + Kind: "Deployment", + }, + { + Name: fmt.Sprintf("%s.%s", serverName, serverNamespace), + Kind: serviceidentity.KindService, + }, + { + Type: otterizev1alpha3.IntentTypeInternet, + Internet: &otterizev1alpha3.Internet{Ips: []string{"8.8.8.8/32"}}, + }, + }, + } + + s.expectGetAllEffectivePolicies([]otterizev1alpha3.ClientIntents{{Spec: intentsSpec, ObjectMeta: metav1.ObjectMeta{Name: namespacedName.Name, Namespace: namespacedName.Namespace}}}) + + // Search for existing NetworkPolicy + emptyNetworkPolicy := &v1.NetworkPolicy{} + networkPolicyNamespacedNameClient := types.NamespacedName{ + Namespace: testNamespace, + Name: "test-client-deployment-access", + } + networkPolicyNamespacedNameServer := types.NamespacedName{ + Namespace: serverNamespace, + Name: "test-server-deployment-access", + } + networkPolicyNamespacedNameServerService := types.NamespacedName{ + Namespace: serverNamespace, + Name: "test-server-service-access", + } + + // Expect GET for the client/server policy names - both return NotFound + s.Client.EXPECT().Get(gomock.Any(), networkPolicyNamespacedNameClient, gomock.Eq(emptyNetworkPolicy)).DoAndReturn( + func(ctx context.Context, name types.NamespacedName, networkPolicy *v1.NetworkPolicy, options ...client.ListOption) error { + return apierrors.NewNotFound(v1.Resource("networkpolicy"), name.Name) + }) + + s.Client.EXPECT().Get(gomock.Any(), networkPolicyNamespacedNameServer, gomock.Eq(emptyNetworkPolicy)).DoAndReturn( + func(ctx context.Context, name types.NamespacedName, networkPolicy *v1.NetworkPolicy, options ...client.ListOption) error { + return apierrors.NewNotFound(v1.Resource("networkpolicy"), name.Name) + }) + + s.Client.EXPECT().Get(gomock.Any(), networkPolicyNamespacedNameServerService, gomock.Eq(emptyNetworkPolicy)).DoAndReturn( + func(ctx context.Context, name types.NamespacedName, networkPolicy *v1.NetworkPolicy, options ...client.ListOption) error { + return apierrors.NewNotFound(v1.Resource("networkpolicy"), name.Name) + }) + + serviceSelector := map[string]string{"app": "test-server"} + svc := s.addExpectedKubernetesServiceCall("test-server", serverNamespace, []corev1.ServicePort{{TargetPort: intstr.IntOrString{IntVal: 80}}}, serviceSelector) + + egressRules := []v1.NetworkPolicyEgressRule{ + {To: []v1.NetworkPolicyPeer{{ + PodSelector: &metav1.LabelSelector{MatchLabels: map[string]string{otterizev1alpha3.OtterizeServiceLabelKey: formattedServerWithoutKind}}, + NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{otterizev1alpha3.KubernetesStandardNamespaceNameLabelKey: serverNamespace}}, + }}}, + {Ports: []v1.NetworkPolicyPort{{Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 80}}}, To: []v1.NetworkPolicyPeer{{ + PodSelector: &metav1.LabelSelector{MatchLabels: serviceSelector}, + NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{otterizev1alpha3.KubernetesStandardNamespaceNameLabelKey: serverNamespace}}, + }}}, + {To: []v1.NetworkPolicyPeer{{IPBlock: &v1.IPBlock{CIDR: "8.8.8.8/32"}}}, Ports: make([]v1.NetworkPolicyPort, 0)}} + + egressNetpol := v1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: networkPolicyNamespacedNameClient.Name, + Namespace: networkPolicyNamespacedNameClient.Namespace, + Labels: map[string]string{otterizev1alpha3.OtterizeNetworkPolicy: formattedClientWithKind}, + }, + Spec: v1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{MatchLabels: map[string]string{otterizev1alpha3.OtterizeServiceLabelKey: formattedClientWithoutKind, otterizev1alpha3.OtterizeOwnerKindLabelKey: "Deployment"}}, + PolicyTypes: []v1.PolicyType{v1.PolicyTypeEgress}, + Ingress: make([]v1.NetworkPolicyIngressRule, 0), + Egress: egressRules, + }, + } + s.Client.EXPECT().Create(gomock.Any(), gomock.Eq(&egressNetpol)) + s.externalNetpolHandler.EXPECT().HandlePodsByLabelSelector(gomock.Any(), egressNetpol.Namespace, gomock.Any()) + + ingressRulesNotSVC := []v1.NetworkPolicyIngressRule{ + {From: []v1.NetworkPolicyPeer{{ + PodSelector: &metav1.LabelSelector{MatchLabels: map[string]string{fmt.Sprintf(otterizev1alpha3.OtterizeAccessLabelKey, formattedServerWithKind): "true"}}, + NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{otterizev1alpha3.KubernetesStandardNamespaceNameLabelKey: testNamespace}}, + }}}} + + netpolNotSVC := v1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: networkPolicyNamespacedNameServer.Name, + Namespace: networkPolicyNamespacedNameServer.Namespace, + Labels: map[string]string{otterizev1alpha3.OtterizeNetworkPolicy: formattedServerWithKind}, + }, + Spec: v1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + otterizev1alpha3.OtterizeServiceLabelKey: formattedServerWithoutKind, + otterizev1alpha3.OtterizeOwnerKindLabelKey: "Deployment", + }, + }, + PolicyTypes: []v1.PolicyType{v1.PolicyTypeIngress}, + Ingress: ingressRulesNotSVC, + Egress: make([]v1.NetworkPolicyEgressRule, 0), + }, + } + + s.Client.EXPECT().Create(gomock.Any(), gomock.Eq(&netpolNotSVC)) + s.externalNetpolHandler.EXPECT().HandlePodsByLabelSelector(gomock.Any(), netpolNotSVC.Namespace, gomock.Any()) + + ingressRulesToSVC := []v1.NetworkPolicyIngressRule{ + {Ports: []v1.NetworkPolicyPort{{Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 80}}}, From: []v1.NetworkPolicyPeer{{ + PodSelector: &metav1.LabelSelector{MatchLabels: map[string]string{fmt.Sprintf(otterizev1alpha3.OtterizeSvcAccessLabelKey, formattedServerService): "true"}}, + NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{otterizev1alpha3.KubernetesStandardNamespaceNameLabelKey: testNamespace}}, + }}}} + + netpolToSVC := v1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: networkPolicyNamespacedNameServerService.Name, + Namespace: networkPolicyNamespacedNameServerService.Namespace, + Labels: map[string]string{otterizev1alpha3.OtterizeNetworkPolicy: formattedServerService}, + }, + Spec: v1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{MatchLabels: serviceSelector}, + PolicyTypes: []v1.PolicyType{v1.PolicyTypeIngress}, + Ingress: ingressRulesToSVC, + Egress: make([]v1.NetworkPolicyEgressRule, 0), + }, + } + + err := controllerutil.SetOwnerReference(svc, &netpolToSVC, s.scheme) + s.Require().NoError(err) + s.Client.EXPECT().Create(gomock.Any(), gomock.Eq(&netpolToSVC)) + s.externalNetpolHandler.EXPECT().HandlePodsByLabelSelector(gomock.Any(), netpolToSVC.Namespace, gomock.Any()) + + s.ignoreRemoveOrphan() + + res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req) + s.ExpectEventsOrderAndCountDontMatter(consts.ReasonCreatedEgressNetworkPolicies, consts.ReasonCreatedNetworkPolicies) + s.NoError(err) + s.Empty(res) +} + func TestAllBuildersTestSuite(t *testing.T) { suite.Run(t, new(AllBuildersTestSuite)) } diff --git a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/egress_network_policy.go b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/egress_network_policy.go index b3c24026f..cedd16933 100644 --- a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/egress_network_policy.go +++ b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/egress_network_policy.go @@ -33,17 +33,18 @@ func (r *EgressNetworkPolicyBuilder) buildNetworkPolicyEgressRules(ep effectivep if call.IsTargetServerKubernetesService() { continue } + targetServiceIdentity := call.ToServiceIdentity(ep.Service.Namespace) egressRules = append(egressRules, v1.NetworkPolicyEgressRule{ To: []v1.NetworkPolicyPeer{ { PodSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - otterizev1alpha3.OtterizeServiceLabelKey: otterizev1alpha3.GetFormattedOtterizeIdentity(call.GetTargetServerName(), call.GetTargetServerNamespace(ep.Service.Namespace)), + otterizev1alpha3.OtterizeServiceLabelKey: targetServiceIdentity.GetFormattedOtterizeIdentityWithoutKind(), }, }, NamespaceSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - otterizev1alpha3.KubernetesStandardNamespaceNameLabelKey: call.GetTargetServerNamespace(ep.Service.Namespace), + otterizev1alpha3.KubernetesStandardNamespaceNameLabelKey: targetServiceIdentity.Namespace, }, }, }, diff --git a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/ingress_network_policy.go b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/ingress_network_policy.go index bfcd31d36..7d3edb0f5 100644 --- a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/ingress_network_policy.go +++ b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/ingress_network_policy.go @@ -42,7 +42,7 @@ func (r *IngressNetpolBuilder) buildIngressRulesFromServiceEffectivePolicy(ep ef PodSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ fmt.Sprintf( - otterizev1alpha3.OtterizeAccessLabelKey, ep.Service.GetFormattedOtterizeIdentity()): "true", + otterizev1alpha3.OtterizeAccessLabelKey, ep.Service.GetFormattedOtterizeIdentityWithKind()): "true", }, }, NamespaceSelector: &metav1.LabelSelector{ diff --git a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/port_network_policy.go b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/port_network_policy.go index 0d9bef717..bec845ed0 100644 --- a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/port_network_policy.go +++ b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/port_network_policy.go @@ -67,7 +67,7 @@ func (r *PortNetworkPolicyReconciler) buildIngressRulesFromEffectivePolicy(ep ef PodSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ fmt.Sprintf( - otterizev1alpha3.OtterizeSvcAccessLabelKey, ep.Service.GetFormattedOtterizeIdentity()): "true", + otterizev1alpha3.OtterizeSvcAccessLabelKey, ep.Service.GetFormattedOtterizeIdentityWithKind()): "true", }, }, NamespaceSelector: &metav1.LabelSelector{ diff --git a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/port_network_policy_test.go b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/port_network_policy_test.go index 1d31b42a1..632d11b5b 100644 --- a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/port_network_policy_test.go +++ b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/port_network_policy_test.go @@ -46,7 +46,7 @@ func (s *PortNetworkPolicyReconcilerTestSuite) TestNetworkPolicyFinalizerAdded() policyName := "test-server-service-access" serviceName := "test-client" serverNamespace := testNamespace - formattedTargetServer := "svc.test-server-test-namespace-ab42d5" + formattedTargetServer := "test-server-test-namespace-servi-c49181" namespacedName := types.NamespacedName{ Namespace: testNamespace, @@ -180,7 +180,7 @@ func (s *PortNetworkPolicyReconcilerTestSuite) TestCreateNetworkPolicyKubernetes policyName := "test-server-service-access" serviceName := "test-client" serverNamespace := testNamespace - formattedTargetServer := "svc.test-server-test-namespace-ab42d5" + formattedTargetServer := "test-server-test-namespace-servi-c49181" s.testCreateNetworkPolicyForKubernetesService( clientIntentsName, @@ -198,7 +198,7 @@ func (s *PortNetworkPolicyReconcilerTestSuite) TestCreateNetworkPolicyKubernetes policyName := "test-server-service-access" serviceName := "test-client" serverNamespace := testNamespace - formattedTargetServer := "svc.test-server-test-namespace-ab42d5" + formattedTargetServer := "test-server-test-namespace-servi-c49181" s.testCreateNetworkPolicyForKubernetesService( clientIntentsName, @@ -216,7 +216,7 @@ func (s *PortNetworkPolicyReconcilerTestSuite) TestCreateNetworkPolicyNamedTarge policyName := "test-server-service-access" serviceName := "test-client" serverNamespace := testNamespace - formattedTargetServer := "svc.test-server-test-namespace-ab42d5" + formattedTargetServer := "test-server-test-namespace-servi-c49181" s.testCreateNetworkPolicyForKubernetesService( clientIntentsName, @@ -399,7 +399,7 @@ func (s *PortNetworkPolicyReconcilerTestSuite) TestUpdateNetworkPolicyForKuberne policyName := "test-server-service-access" serviceName := "test-client" serverNamespace := testNamespace - formattedTargetServer := "svc.test-server-test-namespace-ab42d5" + formattedTargetServer := "test-server-test-namespace-servi-c49181" namespacedName := types.NamespacedName{ Namespace: testNamespace, diff --git a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/test_base.go b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/test_base.go index 2add520a1..dcc23b201 100644 --- a/src/operator/controllers/intents_reconcilers/networkpolicy/builders/test_base.go +++ b/src/operator/controllers/intents_reconcilers/networkpolicy/builders/test_base.go @@ -7,7 +7,6 @@ import ( mocks "github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/mocks" "github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/networkpolicy" "github.com/otterize/intents-operator/src/operator/effectivepolicy" - "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/otterize/intents-operator/src/shared/testbase" "github.com/sirupsen/logrus" "github.com/spf13/viper" @@ -133,8 +132,8 @@ func (s *RulesBuilderTestSuiteBase) expectGetAllEffectivePolicies(clientIntents services := make(map[string][]otterizev1alpha3.ClientIntents) for _, clientIntent := range clientIntents { for _, intentCall := range clientIntent.GetCallsList() { - serverService := serviceidentity.NewFromIntent(intentCall, clientIntent.Namespace) - services[serverService.GetFormattedOtterizeIdentity()] = append(services[serverService.GetFormattedOtterizeIdentity()], clientIntent) + serverService := intentCall.ToServiceIdentity(clientIntent.Namespace) + services[serverService.GetFormattedOtterizeIdentityWithKind()] = append(services[serverService.GetFormattedOtterizeIdentityWithoutKind()], clientIntent) } } diff --git a/src/operator/controllers/intents_reconcilers/networkpolicy/reconciler.go b/src/operator/controllers/intents_reconcilers/networkpolicy/reconciler.go index f7eb47605..bac8e1ed6 100644 --- a/src/operator/controllers/intents_reconcilers/networkpolicy/reconciler.go +++ b/src/operator/controllers/intents_reconcilers/networkpolicy/reconciler.go @@ -219,7 +219,7 @@ func (r *Reconciler) buildIngressRules(ctx context.Context, ep effectivepolicy.S if len(ep.CalledBy) == 0 || len(r.ingressRuleBuilders) == 0 { return rules, false, nil } - shouldCreatePolicy, err := protected_services.IsServerEnforcementEnabledDueToProtectionOrDefaultState(ctx, r.Client, ep.Service.Name, ep.Service.Namespace, r.EnforcementDefaultState, r.EnforcedNamespaces) + shouldCreatePolicy, err := protected_services.IsServerEnforcementEnabledDueToProtectionOrDefaultState(ctx, r.Client, ep.Service, r.EnforcementDefaultState, r.EnforcedNamespaces) if err != nil { return rules, false, errors.Wrap(err) } @@ -247,26 +247,15 @@ func (r *Reconciler) buildIngressRules(ctx context.Context, ep effectivepolicy.S // A function that builds pod label selector from serviceEffectivePolicy func (r *Reconciler) buildPodLabelSelectorFromServiceEffectivePolicy(ctx context.Context, ep effectivepolicy.ServiceEffectivePolicy) (metav1.LabelSelector, bool, error) { - if ep.Service.Kind == serviceidentity.KindService { - svc := corev1.Service{} - err := r.Get(ctx, types.NamespacedName{Name: ep.Service.Name, Namespace: ep.Service.Namespace}, &svc) - if err != nil { - if k8serrors.IsNotFound(err) { - return metav1.LabelSelector{}, false, nil - } - return metav1.LabelSelector{}, false, errors.Wrap(err) - } - if svc.Spec.Selector == nil { - return metav1.LabelSelector{}, false, fmt.Errorf("service %s/%s has no selector", svc.Namespace, svc.Name) - } - return metav1.LabelSelector{MatchLabels: svc.Spec.Selector}, true, nil + labelsMap, ok, err := otterizev1alpha3.ServiceIdentityToLabelsForWorkloadSelection(ctx, r.Client, ep.Service) + if err != nil { + return metav1.LabelSelector{}, false, errors.Wrap(err) + } + if !ok { + return metav1.LabelSelector{}, false, nil } - return metav1.LabelSelector{ - MatchLabels: map[string]string{ - otterizev1alpha3.OtterizeServiceLabelKey: ep.Service.GetFormattedOtterizeIdentity(), - }, - }, true, nil + return metav1.LabelSelector{MatchLabels: labelsMap}, true, nil } func (r *Reconciler) setNetworkPolicyOwnerReferenceIfNeeded(ctx context.Context, ep effectivepolicy.ServiceEffectivePolicy, netpol *v1.NetworkPolicy) error { @@ -316,7 +305,7 @@ func (r *Reconciler) buildNetworkPolicy(ctx context.Context, ep effectivepolicy. Name: fmt.Sprintf(otterizev1alpha3.OtterizeSingleNetworkPolicyNameTemplate, ep.Service.GetNameWithKind()), Namespace: ep.Service.Namespace, Labels: map[string]string{ - otterizev1alpha3.OtterizeNetworkPolicy: ep.Service.GetFormattedOtterizeIdentity(), + otterizev1alpha3.OtterizeNetworkPolicy: ep.Service.GetFormattedOtterizeIdentityWithKind(), }, }, Spec: v1.NetworkPolicySpec{ diff --git a/src/operator/controllers/intents_reconcilers/pods.go b/src/operator/controllers/intents_reconcilers/pods.go index db22e383c..fa2d09861 100644 --- a/src/operator/controllers/intents_reconcilers/pods.go +++ b/src/operator/controllers/intents_reconcilers/pods.go @@ -7,8 +7,8 @@ import ( "github.com/otterize/intents-operator/src/prometheus" "github.com/otterize/intents-operator/src/shared/errors" "github.com/otterize/intents-operator/src/shared/injectablerecorder" + "github.com/otterize/intents-operator/src/shared/serviceidresolver" "github.com/sirupsen/logrus" - v1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" @@ -61,28 +61,24 @@ func (r *PodLabelReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c return ctrl.Result{}, nil } - serviceName := intents.GetServiceName() intentLabels := intents.GetIntentsLabelMapping(namespace) + serviceIdentity := intents.ToServiceIdentity() - // List the pods in the namespace and update labels if required - labelSelector, err := intents.BuildPodLabelSelector() + pods, ok, err := serviceidresolver.NewResolver(r.Client).ResolveServiceIdentityToPodSlice(ctx, serviceIdentity) if err != nil { r.RecordWarningEventf(intents, ReasonListPodsFailed, "could not list pods: %s", err.Error()) return ctrl.Result{}, errors.Wrap(err) } - podList := &v1.PodList{} - err = r.List(ctx, podList, - &client.ListOptions{Namespace: namespace}, - client.MatchingLabelsSelector{Selector: labelSelector}) - if err != nil { - return ctrl.Result{}, errors.Wrap(err) + if !ok { + logrus.Debugf("No pods found for service %s", serviceIdentity.Name) + return ctrl.Result{}, nil } - for _, pod := range podList.Items { + for _, pod := range pods { if otterizev1alpha3.IsMissingOtterizeAccessLabels(&pod, intentLabels) { - logrus.Debugf("Updating %s pod labels with new intents", serviceName) - updatedPod := otterizev1alpha3.UpdateOtterizeAccessLabels(pod.DeepCopy(), serviceName, intentLabels) + logrus.Debugf("Updating %s pod labels with new intents", serviceIdentity.Name) + updatedPod := otterizev1alpha3.UpdateOtterizeAccessLabels(pod.DeepCopy(), serviceIdentity, intentLabels) err := r.Patch(ctx, updatedPod, client.MergeFrom(&pod)) if err != nil { r.RecordWarningEventf(intents, ReasonUpdatePodFailed, "could not update pod: %s", err.Error()) @@ -99,31 +95,27 @@ func (r *PodLabelReconciler) removeLabelsFromPods( logrus.Debugf("Unlabeling pods for Otterize service %s", intents.Spec.Service.Name) - labelSelector, err := intents.BuildPodLabelSelector() + si := intents.ToServiceIdentity() + pods, ok, err := serviceidresolver.NewResolver(r.Client).ResolveServiceIdentityToPodSlice(ctx, si) if err != nil { return errors.Wrap(err) } - podList := &v1.PodList{} - err = r.List(ctx, podList, - &client.ListOptions{Namespace: intents.Namespace}, - client.MatchingLabelsSelector{Selector: labelSelector}) - if err != nil { - return errors.Wrap(err) + if !ok { + logrus.Debugf("No pods found for service %s", si.Name) + return nil } // Remove the access label for each intent, for every pod in the list - for _, pod := range podList.Items { + for _, pod := range pods { updatedPod := pod.DeepCopy() if updatedPod.Annotations == nil { updatedPod.Annotations = make(map[string]string) } updatedPod.Annotations[otterizev1alpha3.AllIntentsRemovedAnnotation] = "true" for _, intent := range intents.GetCallsList() { - targetServerIdentity := otterizev1alpha3.GetFormattedOtterizeIdentity( - intent.Name, intent.GetTargetServerNamespace(intents.Namespace)) - - accessLabel := fmt.Sprintf(otterizev1alpha3.OtterizeAccessLabelKey, targetServerIdentity) + targetServerIdentity := intent.ToServiceIdentity(intents.Namespace) + accessLabel := fmt.Sprintf(otterizev1alpha3.OtterizeAccessLabelKey, targetServerIdentity.GetFormattedOtterizeIdentityWithoutKind()) delete(updatedPod.Labels, accessLabel) } diff --git a/src/operator/controllers/intents_reconcilers/pods_test.go b/src/operator/controllers/intents_reconcilers/pods_test.go index a9aad32a1..d4329764e 100644 --- a/src/operator/controllers/intents_reconcilers/pods_test.go +++ b/src/operator/controllers/intents_reconcilers/pods_test.go @@ -10,7 +10,6 @@ import ( "go.uber.org/mock/gomock" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -73,11 +72,10 @@ func (s *PodLabelReconcilerTestSuite) TestClientAccessLabelAdded() { intents.Spec = &intentsSpec listOption := &client.ListOptions{Namespace: testNamespace} - labelSelector := labels.SelectorFromSet(map[string]string{ - "intents.otterize.com/service": "test-client-test-namespace-537e87", - }) - labelMatcher := client.MatchingLabelsSelector{Selector: labelSelector} + labelMatcher := map[string]string{ + "intents.otterize.com/service": "test-client-test-namespace-537e87", + } pod := v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pod", @@ -138,7 +136,7 @@ func (s *PodLabelReconcilerTestSuite) TestClientAccessLabelAddedTruncatedNameAnd s.Client.EXPECT().Get(gomock.Any(), req.NamespacedName, gomock.Eq(emptyIntents)).DoAndReturn( func(ctx context.Context, name types.NamespacedName, intents *otterizev1alpha3.ClientIntents, options ...client.ListOption) error { intents.Spec = &intentsSpec - intents.Namespace = testNamespace + intents.Namespace = longNamespace return nil }) @@ -146,11 +144,10 @@ func (s *PodLabelReconcilerTestSuite) TestClientAccessLabelAddedTruncatedNameAnd intents.Spec = &intentsSpec listOption := &client.ListOptions{Namespace: longNamespace} - labelSelector := labels.SelectorFromSet(map[string]string{ - "intents.otterize.com/service": "test-client-with-a-v-test-namespace-14e99d", - }) - labelMatcher := client.MatchingLabelsSelector{Selector: labelSelector} + labelMatcher := map[string]string{ + "intents.otterize.com/service": "test-client-with-a-v-test-namespace-with--c115d1", + } pod := v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pod", @@ -171,7 +168,7 @@ func (s *PodLabelReconcilerTestSuite) TestClientAccessLabelAddedTruncatedNameAnd Namespace: testNamespace, Labels: map[string]string{ "intents.otterize.com/access-test-server-test-namespace-with--a1ac14": "true", - "intents.otterize.com/client": "test-client-with-a-v-test-namespace-14e99d", + "intents.otterize.com/client": "test-client-with-a-v-test-namespace-with--c115d1", }, }, Spec: v1.PodSpec{}, @@ -229,11 +226,10 @@ func (s *PodLabelReconcilerTestSuite) testClientAccessLabelRemovedWithParams(pod // Now the reconciler should handle the deletion of the client intents listOption := &client.ListOptions{Namespace: testNamespace} - labelSelector := labels.SelectorFromSet(map[string]string{ - "intents.otterize.com/service": "test-client-test-namespace-537e87", - }) - labelMatcher := client.MatchingLabelsSelector{Selector: labelSelector} + labelMatcher := map[string]string{ + "intents.otterize.com/service": "test-client-test-namespace-537e87", + } pod := v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pod", @@ -310,11 +306,7 @@ func (s *PodLabelReconcilerTestSuite) TestAccessLabelChangedOnIntentsEdit() { intents.Namespace = testNamespace listOption := &client.ListOptions{Namespace: testNamespace} - labelSelector := labels.SelectorFromSet(map[string]string{ - "intents.otterize.com/service": "test-client-test-namespace-537e87", - }) - labelMatcher := client.MatchingLabelsSelector{Selector: labelSelector} pod := v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pod", @@ -323,11 +315,13 @@ func (s *PodLabelReconcilerTestSuite) TestAccessLabelChangedOnIntentsEdit() { }, } - s.Client.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Eq(listOption), gomock.Eq(labelMatcher)).DoAndReturn( + s.Client.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Eq(listOption), gomock.Eq(client.MatchingLabels(map[string]string{ + "intents.otterize.com/service": "test-client-test-namespace-537e87", + }))).DoAndReturn( func(ctx context.Context, pds *v1.PodList, opts ...client.ListOption) error { pds.Items = append(pds.Items, pod) return nil - }) + }).Times(2) updatedPod := v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -359,12 +353,6 @@ func (s *PodLabelReconcilerTestSuite) TestAccessLabelChangedOnIntentsEdit() { return nil }) - s.Client.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Eq(listOption), gomock.Eq(labelMatcher)).DoAndReturn( - func(ctx context.Context, pds *v1.PodList, opts ...client.ListOption) error { - pds.Items = append(pds.Items, pod) - return nil - }) - updatedPod = v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pod", @@ -493,11 +481,10 @@ func (s *PodLabelReconcilerTestSuite) TestClientAccessLabelAddFailedPatch() { intents.Spec = &intentsSpec listOption := &client.ListOptions{Namespace: testNamespace} - labelSelector := labels.SelectorFromSet(map[string]string{ - "intents.otterize.com/service": "test-client-test-namespace-537e87", - }) - labelMatcher := client.MatchingLabelsSelector{Selector: labelSelector} + labelMatcher := map[string]string{ + "intents.otterize.com/service": "test-client-test-namespace-537e87", + } pod := v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pod", diff --git a/src/operator/controllers/intents_reconcilers/port_network_policy/svc_reconcilers.go b/src/operator/controllers/intents_reconcilers/port_network_policy/svc_reconcilers.go index b31f8ffe7..a790934a5 100644 --- a/src/operator/controllers/intents_reconcilers/port_network_policy/svc_reconcilers.go +++ b/src/operator/controllers/intents_reconcilers/port_network_policy/svc_reconcilers.go @@ -2,11 +2,13 @@ package port_network_policy import ( "context" + "github.com/otterize/intents-operator/src/operator/controllers/protected_service_reconcilers" "github.com/otterize/intents-operator/src/operator/effectivepolicy" "github.com/otterize/intents-operator/src/shared/errors" "github.com/otterize/intents-operator/src/shared/injectablerecorder" "github.com/samber/lo" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -19,23 +21,35 @@ type ServiceWatcher struct { client.Client injectablerecorder.InjectableRecorder serviceEffectivePolicyReconciler *effectivepolicy.GroupReconciler + defaultDenyReconciler *protected_service_reconcilers.DefaultDenyReconciler } -func NewServiceWatcher(c client.Client, eventRecorder record.EventRecorder, serviceEffectivePolicyReconciler *effectivepolicy.GroupReconciler) *ServiceWatcher { +func NewServiceWatcher(c client.Client, eventRecorder record.EventRecorder, serviceEffectivePolicyReconciler *effectivepolicy.GroupReconciler, netpolEnabled bool, externalHandler protected_service_reconcilers.ExternalNepolHandler) *ServiceWatcher { recorder := injectablerecorder.InjectableRecorder{Recorder: eventRecorder} - return &ServiceWatcher{ + sw := &ServiceWatcher{ Client: c, InjectableRecorder: recorder, serviceEffectivePolicyReconciler: serviceEffectivePolicyReconciler, } + if netpolEnabled { + sw.defaultDenyReconciler = protected_service_reconcilers.NewDefaultDenyReconciler(c, externalHandler, netpolEnabled) + } + return sw } -func (r *ServiceWatcher) Reconcile(ctx context.Context, _ reconcile.Request) (ctrl.Result, error) { +func (r *ServiceWatcher) Reconcile(ctx context.Context, req reconcile.Request) (ctrl.Result, error) { err := r.serviceEffectivePolicyReconciler.Reconcile(ctx) if err != nil { return ctrl.Result{}, errors.Wrap(err) } + if r.defaultDenyReconciler != nil { + res, err := r.defaultDenyReconciler.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: req.Namespace}}) + if err != nil || res.Requeue { + return res, errors.Wrap(err) + } + } + return ctrl.Result{}, nil } diff --git a/src/operator/controllers/intents_reconcilers/protected_services/should_protect.go b/src/operator/controllers/intents_reconcilers/protected_services/should_protect.go index 75aa8eabd..8bd6bc557 100644 --- a/src/operator/controllers/intents_reconcilers/protected_services/should_protect.go +++ b/src/operator/controllers/intents_reconcilers/protected_services/should_protect.go @@ -5,13 +5,14 @@ import ( "github.com/amit7itz/goset" otterizev1alpha3 "github.com/otterize/intents-operator/src/operator/api/v1alpha3" "github.com/otterize/intents-operator/src/shared/errors" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/sirupsen/logrus" k8serrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) -func IsServerEnforcementEnabledDueToProtectionOrDefaultState(ctx context.Context, kube client.Client, serverName string, serverNamespace string, enforcementDefaultState bool, activeNamespaces *goset.Set[string]) (bool, error) { +func IsServerEnforcementEnabledDueToProtectionOrDefaultState(ctx context.Context, kube client.Client, serverServiceId serviceidentity.ServiceIdentity, enforcementDefaultState bool, activeNamespaces *goset.Set[string]) (bool, error) { if enforcementDefaultState { logrus.Debug("Enforcement is default on, so all services should be protected") return true, nil @@ -19,16 +20,16 @@ func IsServerEnforcementEnabledDueToProtectionOrDefaultState(ctx context.Context logrus.Debug("Protected services are enabled") logrus.Debugf("checking if server's namespace is in acrive namespaces") - if activeNamespaces != nil && activeNamespaces.Contains(serverNamespace) { - logrus.Debugf("Server %s in namespace %s is in active namespaces", serverName, serverNamespace) + if activeNamespaces != nil && activeNamespaces.Contains(serverServiceId.Namespace) { + logrus.Debugf("Server %s in namespace %s is in active namespaces", serverServiceId.Name, serverServiceId.Namespace) return true, nil } logrus.Debugf("checking if server is in protected list") var protectedServicesResources otterizev1alpha3.ProtectedServiceList err := kube.List(ctx, &protectedServicesResources, - client.MatchingFields{otterizev1alpha3.OtterizeProtectedServiceNameIndexField: serverName}, - client.InNamespace(serverNamespace)) + client.MatchingFields{otterizev1alpha3.OtterizeProtectedServiceNameIndexField: serverServiceId.Name}, + client.InNamespace(serverServiceId.Namespace)) if err != nil { if k8serrors.IsNotFound(err) { return false, nil @@ -36,12 +37,16 @@ func IsServerEnforcementEnabledDueToProtectionOrDefaultState(ctx context.Context return false, errors.Wrap(err) } - if len(protectedServicesResources.Items) != 0 { - logrus.Debugf("Server %s in namespace %s is in protected list", serverName, serverNamespace) + // we need to find at least one protected service to have the same kind (or no kind) as the serviceIdentity to enforce + for _, ps := range protectedServicesResources.Items { + if serverServiceId.Kind != "" && ps.Spec.Kind != "" && ps.Spec.Kind != serverServiceId.Kind { + continue + } + logrus.Debugf("Server %s in namespace %s is in protected list", serverServiceId.Name, serverServiceId.Namespace) return true, nil } - logrus.Debugf("Server %s in namespace %s is not in protected list", serverName, serverNamespace) + logrus.Debugf("Server %s in namespace %s is not in protected list", serverServiceId.Name, serverServiceId.Namespace) return false, nil } diff --git a/src/operator/controllers/istiopolicy/policy_manager.go b/src/operator/controllers/istiopolicy/policy_manager.go index 4de1ea076..77a41c906 100644 --- a/src/operator/controllers/istiopolicy/policy_manager.go +++ b/src/operator/controllers/istiopolicy/policy_manager.go @@ -55,6 +55,7 @@ type PolicyManager interface { Create(ctx context.Context, clientIntents *v1alpha3.ClientIntents, clientServiceAccount string) error UpdateIntentsStatus(ctx context.Context, clientIntents *v1alpha3.ClientIntents, clientServiceAccount string, missingSideCar bool) error UpdateServerSidecar(ctx context.Context, clientIntents *v1alpha3.ClientIntents, serverName string, missingSideCar bool) error + RemoveDeprecatedPoliciesForClient(ctx context.Context, clientIntents *v1alpha3.ClientIntents) error } func NewPolicyManager(client client.Client, recorder *injectablerecorder.InjectableRecorder, restrictedNamespaces []string, enforcementDefaultState bool, istioEnforcementEnabled bool, activeNamespaces *goset.Set[string]) *PolicyManagerImpl { @@ -72,16 +73,46 @@ func (c *PolicyManagerImpl) DeleteAll( ctx context.Context, clientIntents *v1alpha3.ClientIntents, ) error { - clientFormattedIdentity := v1alpha3.GetFormattedOtterizeIdentity(clientIntents.Spec.Service.Name, clientIntents.Namespace) + clientServiceIdentity := clientIntents.ToServiceIdentity() + clientFormattedIdentity := clientServiceIdentity.GetFormattedOtterizeIdentityWithKind() var existingPolicies v1beta1.AuthorizationPolicyList err := c.client.List(ctx, &existingPolicies, - client.MatchingLabels{v1alpha3.OtterizeIstioClientAnnotationKey: clientFormattedIdentity}) + client.MatchingLabels{v1alpha3.OtterizeIstioClientWithKindLabelKey: clientFormattedIdentity}) if client.IgnoreNotFound(err) != nil { return errors.Wrap(err) } + for _, policy := range existingPolicies.Items { + err = c.client.Delete(ctx, policy) + if err != nil { + return errors.Wrap(err) + } + } + err = c.RemoveDeprecatedPoliciesForClient(ctx, clientIntents) + if err != nil { + return errors.Wrap(err) + + } + return nil +} + +func (c *PolicyManagerImpl) RemoveDeprecatedPoliciesForClient( + ctx context.Context, + clientIntents *v1alpha3.ClientIntents, +) error { + clientServiceIdentity := clientIntents.ToServiceIdentity() + clientFormattedIdentity := clientServiceIdentity.GetFormattedOtterizeIdentityWithoutKind() + + var existingPolicies v1beta1.AuthorizationPolicyList + err := c.client.List(ctx, + &existingPolicies, + client.MatchingLabels{v1alpha3.OtterizeIstioClientAnnotationKeyDeprecated: clientFormattedIdentity}) + if err != nil { + return errors.Wrap(err) + } + for _, policy := range existingPolicies.Items { err = c.client.Delete(ctx, policy) if err != nil { @@ -96,12 +127,11 @@ func (c *PolicyManagerImpl) Create( clientIntents *v1alpha3.ClientIntents, clientServiceAccount string, ) error { - clientFormattedIdentity := v1alpha3.GetFormattedOtterizeIdentity(clientIntents.Spec.Service.Name, clientIntents.Namespace) - + clientServiceIdentity := clientIntents.ToServiceIdentity() var existingPolicies v1beta1.AuthorizationPolicyList err := c.client.List(ctx, &existingPolicies, - client.MatchingLabels{v1alpha3.OtterizeIstioClientAnnotationKey: clientFormattedIdentity}) + client.MatchingLabels{v1alpha3.OtterizeIstioClientWithKindLabelKey: clientServiceIdentity.GetFormattedOtterizeIdentityWithKind()}) if err != nil { c.recorder.RecordWarningEventf(clientIntents, ReasonGettingIstioPolicyFailed, "Could not get Istio policies: %s", err.Error()) return errors.Wrap(err) @@ -319,11 +349,12 @@ func (c *PolicyManagerImpl) createOrUpdatePolicies( updatedPolicies := goset.NewSet[PolicyID]() createdAnyPolicies := false for _, intent := range clientIntents.GetCallsList() { - if intent.Type != "" && intent.Type != v1alpha3.IntentTypeHTTP || intent.IsTargetServerKubernetesService() { + if intent.Type != "" && intent.Type != v1alpha3.IntentTypeHTTP { continue } + si := intent.ToServiceIdentity(clientIntents.Namespace) shouldCreatePolicy, err := protected_services.IsServerEnforcementEnabledDueToProtectionOrDefaultState( - ctx, c.client, intent.GetTargetServerName(), intent.GetTargetServerNamespace(clientIntents.Namespace), c.enforcementDefaultState, c.activeNamespaces) + ctx, c.client, si, c.enforcementDefaultState, c.activeNamespaces) if err != nil { return nil, errors.Wrap(err) } @@ -350,7 +381,13 @@ func (c *PolicyManagerImpl) createOrUpdatePolicies( continue } - newPolicy := c.generateAuthorizationPolicy(clientIntents, intent, clientServiceAccount) + newPolicy, shouldCreate, err := c.generateAuthorizationPolicy(ctx, clientIntents, intent, clientServiceAccount) + if err != nil { + return nil, errors.Wrap(err) + } + if !shouldCreate { + continue + } existingPolicy, found := c.findPolicy(existingPolicies, newPolicy) if found { err := c.updatePolicy(ctx, existingPolicy, newPolicy) @@ -417,8 +454,10 @@ func (c *PolicyManagerImpl) updatePolicy(ctx context.Context, existingPolicy *v1 } func (c *PolicyManagerImpl) getPolicyName(intents *v1alpha3.ClientIntents, intent v1alpha3.Intent) string { - clientName := fmt.Sprintf("%s.%s", intents.GetServiceName(), intents.Namespace) - policyName := fmt.Sprintf(OtterizeIstioPolicyNameTemplate, intent.GetTargetServerName(), clientName) + clientIdentity := intents.ToServiceIdentity() + clientName := fmt.Sprintf("%s.%s", clientIdentity.GetNameWithKind(), intents.Namespace) + serverIdentity := intent.ToServiceIdentity(intents.Namespace) + policyName := fmt.Sprintf(OtterizeIstioPolicyNameTemplate, serverIdentity.GetNameWithKind(), clientName) return policyName } @@ -457,16 +496,19 @@ func compareHTTPRules(existingRules []*v1beta1security.Rule_To, newRules []*v1be } func (c *PolicyManagerImpl) generateAuthorizationPolicy( + ctx context.Context, clientIntents *v1alpha3.ClientIntents, intent v1alpha3.Intent, clientServiceAccountName string, -) *v1beta1.AuthorizationPolicy { +) (*v1beta1.AuthorizationPolicy, bool, error) { policyName := c.getPolicyName(clientIntents, intent) logrus.Debugf("Creating Istio policy %s for intent %s", policyName, intent.GetTargetServerName()) + clientIdentity := clientIntents.ToServiceIdentity() + serverIdentity := intent.ToServiceIdentity(clientIntents.Namespace) serverNamespace := intent.GetTargetServerNamespace(clientIntents.Namespace) - formattedTargetServer := v1alpha3.GetFormattedOtterizeIdentity(intent.GetTargetServerName(), serverNamespace) - clientFormattedIdentity := v1alpha3.GetFormattedOtterizeIdentity(clientIntents.GetServiceName(), clientIntents.Namespace) + formattedTargetServer := serverIdentity.GetFormattedOtterizeIdentityWithKind() + clientFormattedIdentity := clientIdentity.GetFormattedOtterizeIdentityWithKind() var ruleTo []*v1beta1security.Rule_To if intent.Type == v1alpha3.IntentTypeHTTP { @@ -479,21 +521,27 @@ func (c *PolicyManagerImpl) generateAuthorizationPolicy( } } + podSelector, shouldCreate, err := v1alpha3.ServiceIdentityToLabelsForWorkloadSelection(ctx, c.client, serverIdentity) + if err != nil { + return nil, false, errors.Wrap(err) + } + if !shouldCreate { + return nil, false, nil + } + source := fmt.Sprintf("cluster.local/ns/%s/sa/%s", clientIntents.Namespace, clientServiceAccountName) newPolicy := &v1beta1.AuthorizationPolicy{ ObjectMeta: v1.ObjectMeta{ Name: policyName, Namespace: serverNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: formattedTargetServer, - v1alpha3.OtterizeIstioClientAnnotationKey: clientFormattedIdentity, + v1alpha3.OtterizeServiceLabelKey: formattedTargetServer, + v1alpha3.OtterizeIstioClientWithKindLabelKey: clientFormattedIdentity, }, }, Spec: v1beta1security.AuthorizationPolicy{ Selector: &v1beta1type.WorkloadSelector{ - MatchLabels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: formattedTargetServer, - }, + MatchLabels: podSelector, }, Action: v1beta1security.AuthorizationPolicy_ALLOW, Rules: []*v1beta1security.Rule{ @@ -513,7 +561,8 @@ func (c *PolicyManagerImpl) generateAuthorizationPolicy( }, } - return newPolicy + return newPolicy, true, nil + } func (c *PolicyManagerImpl) intentsHTTPResourceToIstioOperations(resources []v1alpha3.HTTPResource) []*v1beta1security.Operation { diff --git a/src/operator/controllers/istiopolicy/policy_manager_test.go b/src/operator/controllers/istiopolicy/policy_manager_test.go index ac21e46e1..e727c0c5a 100644 --- a/src/operator/controllers/istiopolicy/policy_manager_test.go +++ b/src/operator/controllers/istiopolicy/policy_manager_test.go @@ -47,8 +47,8 @@ func (s *PolicyManagerTestSuite) TestCreateProtectedService() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: &v1alpha3.IntentsSpec{ @@ -70,8 +70,8 @@ func (s *PolicyManagerTestSuite) TestCreateProtectedService() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: v1beta12.AuthorizationPolicy{ @@ -130,8 +130,8 @@ func (s *PolicyManagerTestSuite) TestCreateEnforcementDisabledNoProtectedService Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: &v1alpha3.IntentsSpec{ @@ -170,8 +170,8 @@ func (s *PolicyManagerTestSuite) TestCreateIstioEnforcementDisabledNoProtectedSe Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: &v1alpha3.IntentsSpec{ @@ -194,41 +194,6 @@ func (s *PolicyManagerTestSuite) TestCreateIstioEnforcementDisabledNoProtectedSe s.ExpectEvent(consts.ReasonIstioPolicyCreationDisabled) } -func (s *PolicyManagerTestSuite) TestCreateIstioIgnoreK8sServiceIntents() { - s.admin.enableIstioPolicyCreation = false - clientName := "test-client" - serverName := "svc:otterservice.otternamespace" - policyName := "authorization-policy-to-test-server-from-test-client.test-namespace" - clientIntentsNamespace := "test-namespace" - - intents := &v1alpha3.ClientIntents{ - ObjectMeta: v1.ObjectMeta{ - Name: policyName, - Namespace: clientIntentsNamespace, - Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", - }, - }, - Spec: &v1alpha3.IntentsSpec{ - Service: v1alpha3.Service{ - Name: clientName, - }, - Calls: []v1alpha3.Intent{ - { - Name: serverName, - }, - }, - }, - } - clientServiceAccountName := "test-client-sa" - - s.Client.EXPECT().List(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(client.MatchingLabels{})).Return(nil) - - err := s.admin.Create(context.Background(), intents, clientServiceAccountName) - s.NoError(err) -} - func (s *PolicyManagerTestSuite) TestCreateProtectedServiceIstioEnforcementDisabled() { s.admin.enableIstioPolicyCreation = false clientName := "test-client" @@ -241,8 +206,8 @@ func (s *PolicyManagerTestSuite) TestCreateProtectedServiceIstioEnforcementDisab Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: &v1alpha3.IntentsSpec{ @@ -276,8 +241,8 @@ func (s *PolicyManagerTestSuite) TestCreate() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: &v1alpha3.IntentsSpec{ @@ -299,8 +264,8 @@ func (s *PolicyManagerTestSuite) TestCreate() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: v1beta12.AuthorizationPolicy{ @@ -379,8 +344,8 @@ func (s *PolicyManagerTestSuite) TestCreateHTTPResources() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: v1beta12.AuthorizationPolicy{ @@ -481,8 +446,8 @@ func (s *PolicyManagerTestSuite) TestUpdateHTTPResources() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: v1beta12.AuthorizationPolicy{ @@ -535,8 +500,8 @@ func (s *PolicyManagerTestSuite) TestUpdateHTTPResources() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: v1beta12.AuthorizationPolicy{ @@ -627,8 +592,8 @@ func (s *PolicyManagerTestSuite) TestNothingToUpdateHTTPResources() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: v1beta12.AuthorizationPolicy{ @@ -746,8 +711,8 @@ func (s *PolicyManagerTestSuite) TestNamespaceAllowed() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: v1beta12.AuthorizationPolicy{ @@ -809,8 +774,8 @@ func (s *PolicyManagerTestSuite) TestUpdatePolicy() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: v1beta12.AuthorizationPolicy{ @@ -841,8 +806,8 @@ func (s *PolicyManagerTestSuite) TestUpdatePolicy() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: v1beta12.AuthorizationPolicy{ @@ -913,8 +878,13 @@ func (s *PolicyManagerTestSuite) TestDeleteAllPoliciesForClientIntents() { } authzPol := &v1beta1.AuthorizationPolicy{ObjectMeta: v1.ObjectMeta{Name: "blah"}} + + // Expected removeDeprecatedPolicies + s.Client.EXPECT().List(gomock.Any(), gomock.Any(), client.MatchingLabels{ + v1alpha3.OtterizeIstioClientAnnotationKeyDeprecated: "test-client-test-namespace-537e87", + }) s.Client.EXPECT().List(gomock.Any(), gomock.Any(), client.MatchingLabels{ - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }).SetArg(1, v1beta1.AuthorizationPolicyList{Items: []*v1beta1.AuthorizationPolicy{authzPol}}).Return(nil) s.Client.EXPECT().Delete(gomock.Any(), authzPol).Return(nil) @@ -953,8 +923,8 @@ func (s *PolicyManagerTestSuite) TestNothingToUpdate() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, }, Spec: v1beta12.AuthorizationPolicy{ @@ -1019,8 +989,8 @@ func (s *PolicyManagerTestSuite) TestDeletePolicy() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-test-namespace-8ddecb", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, UID: "uid_1", }, @@ -1051,8 +1021,8 @@ func (s *PolicyManagerTestSuite) TestDeletePolicy() { Name: policyName, Namespace: clientIntentsNamespace, Labels: map[string]string{ - v1alpha3.OtterizeServiceLabelKey: "test-server-from-old-intent-file", - v1alpha3.OtterizeIstioClientAnnotationKey: "test-client-test-namespace-537e87", + v1alpha3.OtterizeServiceLabelKey: "test-server-from-old-intent-file", + v1alpha3.OtterizeIstioClientWithKindLabelKey: "test-client-test-namespace-537e87", }, UID: "uid_2", }, diff --git a/src/operator/controllers/kafka_server_config_reconcilers/kafka_server_config_reconciler.go b/src/operator/controllers/kafka_server_config_reconcilers/kafka_server_config_reconciler.go index 035380a9e..1dda1e5dd 100644 --- a/src/operator/controllers/kafka_server_config_reconcilers/kafka_server_config_reconciler.go +++ b/src/operator/controllers/kafka_server_config_reconcilers/kafka_server_config_reconciler.go @@ -155,6 +155,7 @@ func (r *KafkaServerConfigReconciler) createIntentsFromOperatorToKafkaServer(ctx Calls: []otterizev1alpha3.Intent{{ Type: otterizev1alpha3.IntentTypeKafka, Name: fmt.Sprintf("%s.%s", config.Spec.Service.Name, config.Namespace), + Kind: config.Spec.Service.Kind, Topics: []otterizev1alpha3.KafkaTopic{{ Name: "*", Operations: []otterizev1alpha3.KafkaOperation{ diff --git a/src/operator/controllers/pod_reconcilers/pods.go b/src/operator/controllers/pod_reconcilers/pods.go index 75667c480..2df25e871 100644 --- a/src/operator/controllers/pod_reconcilers/pods.go +++ b/src/operator/controllers/pod_reconcilers/pods.go @@ -30,7 +30,8 @@ import ( ) const ( - OtterizeClientNameIndexField = "spec.service.name" + OtterizeClientNameIndexField = "spec.service.name" + OtterizeClientNameWithKindIndexField = "spec.service.nameWithKind" ) //+kubebuilder:rbac:groups="",resources=pods,verbs=get;update;patch;list;watch @@ -78,17 +79,6 @@ func (p *PodWatcher) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, errors.Wrap(err) } - // If a new pod starts, check if we need to do something for it. - var intents otterizev1alpha3.ClientIntentsList - err = p.List( - ctx, - &intents, - &client.MatchingFields{OtterizeClientNameIndexField: serviceID.Name}, - &client.ListOptions{Namespace: pod.Namespace}) - if err != nil { - return ctrl.Result{}, errors.Wrap(err) - } - err = p.addOtterizePodLabels(ctx, req, serviceID, pod) if err != nil { return ctrl.Result{}, errors.Wrap(err) @@ -99,7 +89,7 @@ func (p *PodWatcher) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, errors.Wrap(err) } - res, err := p.handleDatabaseIntents(ctx, pod, intents) + res, err := p.handleDatabaseIntents(ctx, pod, serviceID) if err != nil { return ctrl.Result{}, errors.Wrap(err) } @@ -131,12 +121,7 @@ func (p *PodWatcher) handleIstioPolicy(ctx context.Context, pod v1.Pod, serviceI return errors.Wrap(err) } - var intents otterizev1alpha3.ClientIntentsList - err = p.List( - ctx, - &intents, - &client.MatchingFields{OtterizeClientNameIndexField: serviceID.Name}, - &client.ListOptions{Namespace: pod.Namespace}) + intents, err := p.getClientIntentsForServiceIdentity(ctx, serviceID) if err != nil { logrus.WithFields(logrus.Fields{"ServiceName": serviceID, "Namespace": pod.Namespace}).Errorln("Failed listing intents") return errors.Wrap(err) @@ -173,8 +158,7 @@ func (p *PodWatcher) updateServerSideCar(ctx context.Context, pod v1.Pod, servic } for _, clientIntents := range intentsList.Items { - formattedTargetServer := otterizev1alpha3.GetFormattedOtterizeIdentity(serviceID.Name, pod.Namespace) - err = p.istioPolicyAdmin.UpdateServerSidecar(ctx, &clientIntents, formattedTargetServer, missingSideCar) + err = p.istioPolicyAdmin.UpdateServerSidecar(ctx, &clientIntents, serviceID.GetFormattedOtterizeIdentityWithoutKind(), missingSideCar) if err != nil { return errors.Wrap(err) } @@ -196,7 +180,7 @@ func (p *PodWatcher) addOtterizePodLabels(ctx context.Context, req ctrl.Request, return nil } - otterizeServerLabelValue := otterizev1alpha3.GetFormattedOtterizeIdentity(serviceID.Name, pod.Namespace) + otterizeServerLabelValue := serviceID.GetFormattedOtterizeIdentityWithoutKind() updatedPod := pod.DeepCopy() hasUpdates := false @@ -212,18 +196,19 @@ func (p *PodWatcher) addOtterizePodLabels(ctx context.Context, req ctrl.Request, hasUpdates = true } + if !otterizev1alpha3.HasOtterizeOwnerKindLabel(&pod, serviceID.Kind) { + logrus.Debugf("Labeling pod %s with owner kind %s", pod.Name, serviceID.Kind) + updatedPod.Labels[otterizev1alpha3.OtterizeOwnerKindLabelKey] = serviceID.Kind + hasUpdates = true + } + if otterizev1alpha3.HasOtterizeDeprecatedServerLabel(&pod) { logrus.Debugf("Removing deprecated label for pod %s with server identity %s", pod.Name, serviceID.Name) delete(updatedPod.Labels, otterizev1alpha3.OtterizeServerLabelKeyDeprecated) hasUpdates = true } - var intents otterizev1alpha3.ClientIntentsList - err := p.List( - ctx, &intents, - &client.MatchingFields{OtterizeClientNameIndexField: serviceID.Name}, - &client.ListOptions{Namespace: pod.Namespace}) - + intents, err := p.getClientIntentsForServiceIdentity(ctx, serviceID) if err != nil { logrus.WithFields(logrus.Fields{"ServiceName": serviceID, "Namespace": pod.Namespace}).Errorln("Failed listing intents") return errors.Wrap(err) @@ -240,7 +225,7 @@ func (p *PodWatcher) addOtterizePodLabels(ctx context.Context, req ctrl.Request, } if otterizev1alpha3.IsMissingOtterizeAccessLabels(&pod, otterizeAccessLabels) { logrus.Debugf("Updating Otterize access labels for %s", serviceID.Name) - updatedPod = otterizev1alpha3.UpdateOtterizeAccessLabels(updatedPod.DeepCopy(), serviceID.Name, otterizeAccessLabels) + updatedPod = otterizev1alpha3.UpdateOtterizeAccessLabels(updatedPod.DeepCopy(), serviceID, otterizeAccessLabels) prometheus.IncrementPodsLabeledForNetworkPolicies(1) hasUpdates = true } @@ -255,6 +240,35 @@ func (p *PodWatcher) addOtterizePodLabels(ctx context.Context, req ctrl.Request, return nil } +func (p *PodWatcher) getClientIntentsForServiceIdentity(ctx context.Context, serviceID serviceidentity.ServiceIdentity) (otterizev1alpha3.ClientIntentsList, error) { + var intents otterizev1alpha3.ClientIntentsList + + // first check if there are intents specifically for this service identity (with kind) + err := p.List( + ctx, &intents, + &client.MatchingFields{OtterizeClientNameWithKindIndexField: serviceID.GetNameWithKind()}, + &client.ListOptions{Namespace: serviceID.Namespace}) + if err != nil { + return otterizev1alpha3.ClientIntentsList{}, errors.Wrap(err) + } + + // If there are specific intents for this service identity, return them + if len(intents.Items) != 0 { + return intents, nil + } + + // list all intents for this service name (without kind) + err = p.List( + ctx, &intents, + &client.MatchingFields{OtterizeClientNameIndexField: serviceID.Name}, + &client.ListOptions{Namespace: serviceID.Namespace}) + + if err != nil { + return otterizev1alpha3.ClientIntentsList{}, errors.Wrap(err) + } + return intents, nil +} + func (p *PodWatcher) istioEnforcementEnabled() bool { return viper.GetBool(operatorconfig.EnableIstioPolicyKey) } @@ -302,6 +316,19 @@ func (p *PodWatcher) InitIntentsClientIndices(mgr manager.Manager) error { return errors.Wrap(err) } + err = mgr.GetCache().IndexField( + context.Background(), + &otterizev1alpha3.ClientIntents{}, + OtterizeClientNameWithKindIndexField, + func(object client.Object) []string { + intents := object.(*otterizev1alpha3.ClientIntents) + serviceIdentity := intents.ToServiceIdentity() + return []string{serviceIdentity.GetNameWithKind()} + }) + if err != nil { + return errors.Wrap(err) + } + return nil } @@ -321,7 +348,7 @@ func (p *PodWatcher) Register(mgr manager.Manager) error { return nil } -func (p *PodWatcher) handleDatabaseIntents(ctx context.Context, pod v1.Pod, clientIntentsList otterizev1alpha3.ClientIntentsList) (ctrl.Result, error) { +func (p *PodWatcher) handleDatabaseIntents(ctx context.Context, pod v1.Pod, serviceID serviceidentity.ServiceIdentity) (ctrl.Result, error) { if pod.Annotations == nil { return ctrl.Result{}, nil } @@ -331,6 +358,11 @@ func (p *PodWatcher) handleDatabaseIntents(ctx context.Context, pod v1.Pod, clie return ctrl.Result{}, nil } + clientIntentsList, err := p.getClientIntentsForServiceIdentity(ctx, serviceID) + if err != nil { + return ctrl.Result{}, errors.Wrap(err) + } + dbIntents := lo.Filter(clientIntentsList.Items, func(clientIntents otterizev1alpha3.ClientIntents, _ int) bool { return len(clientIntents.GetDatabaseIntents()) > 0 }) diff --git a/src/operator/controllers/pod_reconcilers/pods_test.go b/src/operator/controllers/pod_reconcilers/pods_test.go index 7183a4674..85a1b93b7 100644 --- a/src/operator/controllers/pod_reconcilers/pods_test.go +++ b/src/operator/controllers/pod_reconcilers/pods_test.go @@ -5,6 +5,7 @@ import ( "fmt" otterizev1alpha3 "github.com/otterize/intents-operator/src/operator/api/v1alpha3" mocks "github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/mocks" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/otterize/intents-operator/src/shared/testbase" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" @@ -72,10 +73,9 @@ func (s *WatcherPodLabelReconcilerTestSuite) TestServerLabelAddedWithNilLabels() serviceID, err := s.Reconciler.serviceIdResolver.ResolvePodToServiceIdentity(context.Background(), &pod) s.Require().NoError(err) - thisPodIdentity := otterizev1alpha3.GetFormattedOtterizeIdentity( - serviceID.Name, s.TestNamespace) + thisPodIdentity := (&serviceidentity.ServiceIdentity{Name: serviceID.Name, Namespace: s.TestNamespace}).GetFormattedOtterizeIdentityWithoutKind() - _, err = s.AddIntents("test-intents", serviceID.Name, []otterizev1alpha3.Intent{{ + _, err = s.AddIntents("test-intents", serviceID.Name, "", []otterizev1alpha3.Intent{{ Type: otterizev1alpha3.IntentTypeHTTP, Name: intentTargetServerName, }, }) @@ -111,8 +111,7 @@ func (s *WatcherPodLabelReconcilerTestSuite) TestServerLabelAddedWithNilLabels() s.Require().NoError(err) s.Require().Empty(res) - targetServerIdentity := otterizev1alpha3.GetFormattedOtterizeIdentity( - intentTargetServerName, s.TestNamespace) + targetServerIdentity := (&serviceidentity.ServiceIdentity{Name: intentTargetServerName, Namespace: s.TestNamespace}).GetFormattedOtterizeIdentityWithoutKind() accessLabel := fmt.Sprintf(otterizev1alpha3.OtterizeAccessLabelKey, targetServerIdentity) s.WaitUntilCondition(func(assert *assert.Assertions) { err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{ @@ -133,7 +132,7 @@ func (s *WatcherPodLabelReconcilerTestSuite) TestClientAccessLabelAdded() { map[string]string{"someLabel": "cake"}, map[string]string{}) - _, err := s.AddIntents("test-intents", deploymentName, []otterizev1alpha3.Intent{{ + _, err := s.AddIntents("test-intents", deploymentName, "Deployment", []otterizev1alpha3.Intent{{ Type: otterizev1alpha3.IntentTypeHTTP, Name: intentTargetServerName, }, }) @@ -159,8 +158,7 @@ func (s *WatcherPodLabelReconcilerTestSuite) TestClientAccessLabelAdded() { serviceID, err := s.Reconciler.serviceIdResolver.ResolvePodToServiceIdentity(context.Background(), &pod) s.Require().NoError(err) - thisPodIdentity := otterizev1alpha3.GetFormattedOtterizeIdentity( - serviceID.Name, s.TestNamespace) + thisPodIdentity := (&serviceidentity.ServiceIdentity{Name: serviceID.Name, Namespace: s.TestNamespace}).GetFormattedOtterizeIdentityWithoutKind() s.WaitUntilCondition(func(assert *assert.Assertions) { err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{ @@ -180,8 +178,7 @@ func (s *WatcherPodLabelReconcilerTestSuite) TestClientAccessLabelAdded() { }, }) - targetServerIdentity := otterizev1alpha3.GetFormattedOtterizeIdentity( - intentTargetServerName, s.TestNamespace) + targetServerIdentity := (&serviceidentity.ServiceIdentity{Name: intentTargetServerName, Namespace: s.TestNamespace}).GetFormattedOtterizeIdentityWithoutKind() accessLabel := fmt.Sprintf(otterizev1alpha3.OtterizeAccessLabelKey, targetServerIdentity) s.WaitUntilCondition(func(assert *assert.Assertions) { diff --git a/src/operator/controllers/protected_service_reconcilers/default_deny.go b/src/operator/controllers/protected_service_reconcilers/default_deny.go index d82eeef01..b9c9ed23b 100644 --- a/src/operator/controllers/protected_service_reconcilers/default_deny.go +++ b/src/operator/controllers/protected_service_reconcilers/default_deny.go @@ -6,6 +6,7 @@ import ( otterizev1alpha3 "github.com/otterize/intents-operator/src/operator/api/v1alpha3" "github.com/otterize/intents-operator/src/shared/errors" "github.com/otterize/intents-operator/src/shared/injectablerecorder" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/sirupsen/logrus" v1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -67,8 +68,15 @@ func (r *DefaultDenyReconciler) blockAccessToServices(ctx context.Context, prote continue } - formattedServerName := otterizev1alpha3.GetFormattedOtterizeIdentity(protectedService.Spec.Name, namespace) - policy := r.buildNetworkPolicyObjectForIntent(formattedServerName, protectedService.Spec.Name, namespace) + serverServiceIdentity := protectedService.ToServiceIdentity() + formattedServerName := serverServiceIdentity.GetFormattedOtterizeIdentityWithKind() + policy, shouldCreate, err := r.buildNetworkPolicyObjectForIntent(ctx, serverServiceIdentity) + if err != nil { + return errors.Wrap(err) + } + if !shouldCreate { + continue + } if r.netpolEnforcementEnabled { serversToProtect[formattedServerName] = policy } @@ -132,31 +140,34 @@ func (r *DefaultDenyReconciler) updateIfNeeded( return nil } -func (r *DefaultDenyReconciler) buildNetworkPolicyObjectForIntent( - formattedServerName string, - serviceName string, - namespace string, -) v1.NetworkPolicy { - policyName := fmt.Sprintf("default-deny-%s", serviceName) - return v1.NetworkPolicy{ +func (r *DefaultDenyReconciler) buildNetworkPolicyObjectForIntent(ctx context.Context, serviceId serviceidentity.ServiceIdentity) (v1.NetworkPolicy, bool, error) { + policyName := fmt.Sprintf("default-deny-%s", serviceId.GetNameWithKind()) + podSelectorLabels, ok, err := otterizev1alpha3.ServiceIdentityToLabelsForWorkloadSelection(ctx, r.Client, serviceId) + if err != nil { + return v1.NetworkPolicy{}, false, errors.Wrap(err) + } + if !ok { + return v1.NetworkPolicy{}, false, nil + } + netpol := v1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: policyName, - Namespace: namespace, + Namespace: serviceId.Namespace, Labels: map[string]string{ otterizev1alpha3.OtterizeNetworkPolicyServiceDefaultDeny: "true", - otterizev1alpha3.OtterizeNetworkPolicy: formattedServerName, + otterizev1alpha3.OtterizeNetworkPolicy: serviceId.GetFormattedOtterizeIdentityWithoutKind(), }, }, Spec: v1.NetworkPolicySpec{ PolicyTypes: []v1.PolicyType{v1.PolicyTypeIngress}, PodSelector: metav1.LabelSelector{ - MatchLabels: map[string]string{ - otterizev1alpha3.OtterizeServiceLabelKey: formattedServerName, - }, + MatchLabels: podSelectorLabels, }, Ingress: []v1.NetworkPolicyIngressRule{}, }, } + + return netpol, true, nil } func (r *DefaultDenyReconciler) DeleteAllDefaultDeny(ctx context.Context, namespace string) (ctrl.Result, error) { diff --git a/src/operator/controllers/protected_service_reconcilers/default_deny_test.go b/src/operator/controllers/protected_service_reconcilers/default_deny_test.go index ca31453c9..96d291e67 100644 --- a/src/operator/controllers/protected_service_reconcilers/default_deny_test.go +++ b/src/operator/controllers/protected_service_reconcilers/default_deny_test.go @@ -9,6 +9,7 @@ import ( "github.com/spf13/viper" "github.com/stretchr/testify/suite" "go.uber.org/mock/gomock" + corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -156,6 +157,84 @@ func (s *DefaultDenyReconcilerTestSuite) TestProtectedServicesCreate() { s.Require().NoError(err) } +func (s *DefaultDenyReconcilerTestSuite) TestProtectedServicesCreate_KindService() { + var protectedServicesResources otterizev1alpha3.ProtectedServiceList + protectedServicesResources.Items = []otterizev1alpha3.ProtectedService{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: protectedServicesResourceName, + Namespace: testNamespace, + }, + Spec: otterizev1alpha3.ProtectedServiceSpec{ + Name: protectedServiceName, + Kind: "Service", + }, + }, + } + + k8sService := corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: protectedServiceName, + Namespace: testNamespace, + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": "test"}, + }, + } + + s.Client.EXPECT().Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: protectedServiceName, Namespace: testNamespace}), gomock.Any()).Do( + func(ctx context.Context, name types.NamespacedName, service *corev1.Service, _ ...any) error { + k8sService.DeepCopyInto(service) + return nil + }) + + s.Client.EXPECT().List(gomock.Any(), gomock.Eq(&otterizev1alpha3.ProtectedServiceList{}), client.InNamespace(testNamespace)).DoAndReturn( + func(ctx context.Context, list *otterizev1alpha3.ProtectedServiceList, opts ...client.ListOption) error { + protectedServicesResources.DeepCopyInto(list) + return nil + }) + + request := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: testNamespace, + Name: protectedServicesResourceName, + }, + } + + // Get all existing network policies + // No network policies exist + var networkPolicies v1.NetworkPolicyList + s.Client.EXPECT().List(gomock.Any(), gomock.Eq(&networkPolicies), client.InNamespace(testNamespace), client.MatchingLabels{ + otterizev1alpha3.OtterizeNetworkPolicyServiceDefaultDeny: "true", + }).Return(nil).Times(1) + + // Create network policy + formattedServerName := protectedServiceFormattedName + policy := v1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default-deny-test-service-service", + Namespace: testNamespace, + Labels: map[string]string{ + otterizev1alpha3.OtterizeNetworkPolicyServiceDefaultDeny: "true", + otterizev1alpha3.OtterizeNetworkPolicy: formattedServerName, + }, + }, + Spec: v1.NetworkPolicySpec{ + PolicyTypes: []v1.PolicyType{v1.PolicyTypeIngress}, + PodSelector: metav1.LabelSelector{ + MatchLabels: k8sService.Spec.Selector, + }, + Ingress: []v1.NetworkPolicyIngressRule{}, + }, + } + s.Client.EXPECT().Create(gomock.Any(), gomock.Eq(&policy)).Return(nil).Times(1) + + s.extNetpolHandler.EXPECT().HandleAllPods(gomock.Any()) + res, err := s.reconciler.Reconcile(context.Background(), request) + s.Require().Empty(res) + s.Require().NoError(err) +} + func (s *DefaultDenyReconcilerTestSuite) TestProtectedServicesCreateFromMultipleLists() { var protectedServicesResources otterizev1alpha3.ProtectedServiceList protectedServicesResources.Items = []otterizev1alpha3.ProtectedService{ diff --git a/src/operator/effectivepolicy/groupreconciler.go b/src/operator/effectivepolicy/groupreconciler.go index 07fed93aa..61c03a7a2 100644 --- a/src/operator/effectivepolicy/groupreconciler.go +++ b/src/operator/effectivepolicy/groupreconciler.go @@ -79,14 +79,14 @@ func (g *GroupReconciler) getAllServiceEffectivePolicies(ctx context.Context) ([ if !clientIntent.DeletionTimestamp.IsZero() { continue } - service := serviceidentity.NewFromClientIntent(clientIntent) + service := clientIntent.ToServiceIdentity() services.Add(service) serviceToIntent[service] = clientIntent for _, intentCall := range clientIntent.GetCallsList() { if !g.shouldCreateEffectivePolicyForIntentTargetServer(intentCall, clientIntent.Namespace) { continue } - services.Add(serviceidentity.NewFromIntent(intentCall, clientIntent.Namespace)) + services.Add(intentCall.ToServiceIdentity(clientIntent.Namespace)) } } @@ -157,7 +157,7 @@ func (g *GroupReconciler) filterAndTransformClientIntentsIntoClientCalls(clientI func (g *GroupReconciler) getClientIntentsByServer(ctx context.Context, server serviceidentity.ServiceIdentity) ([]v1alpha3.ClientIntents, error) { var intentsList v1alpha3.ClientIntentsList - matchFields := client.MatchingFields{v1alpha3.OtterizeFormattedTargetServerIndexField: server.GetFormattedOtterizeIdentity()} + matchFields := client.MatchingFields{v1alpha3.OtterizeFormattedTargetServerIndexField: server.GetFormattedOtterizeIdentityWithKind()} err := g.Client.List( ctx, &intentsList, &matchFields, diff --git a/src/operator/main.go b/src/operator/main.go index 3e5af0639..255600098 100644 --- a/src/operator/main.go +++ b/src/operator/main.go @@ -463,7 +463,7 @@ func main() { podWatcher := pod_reconcilers.NewPodWatcher(mgr.GetClient(), mgr.GetEventRecorderFor("intents-operator"), watchedNamespaces, enforcementConfig.EnforcementDefaultState, enforcementConfig.EnableIstioPolicy, enforcementConfig.EnforcedNamespaces, intentsReconciler) nsWatcher := pod_reconcilers.NewNamespaceWatcher(mgr.GetClient()) - svcWatcher := port_network_policy.NewServiceWatcher(mgr.GetClient(), mgr.GetEventRecorderFor("intents-operator"), epGroupReconciler) + svcWatcher := port_network_policy.NewServiceWatcher(mgr.GetClient(), mgr.GetEventRecorderFor("intents-operator"), epGroupReconciler, enforcementConfig.EnableNetworkPolicy, extNetpolHandler) err = svcWatcher.SetupWithManager(mgr) if err != nil { diff --git a/src/operator/otterizecrds/clientintents-customresourcedefinition.yaml b/src/operator/otterizecrds/clientintents-customresourcedefinition.yaml index edcf6395a..1a2cb0e53 100644 --- a/src/operator/otterizecrds/clientintents-customresourcedefinition.yaml +++ b/src/operator/otterizecrds/clientintents-customresourcedefinition.yaml @@ -372,6 +372,8 @@ spec: - operations type: object type: array + kind: + type: string name: type: string type: @@ -388,6 +390,8 @@ spec: type: array service: properties: + kind: + type: string name: type: string required: diff --git a/src/operator/otterizecrds/kafkaserverconfigs-customresourcedefinition.yaml b/src/operator/otterizecrds/kafkaserverconfigs-customresourcedefinition.yaml index 935498a07..13e8d66b9 100644 --- a/src/operator/otterizecrds/kafkaserverconfigs-customresourcedefinition.yaml +++ b/src/operator/otterizecrds/kafkaserverconfigs-customresourcedefinition.yaml @@ -144,6 +144,8 @@ spec: type: boolean service: properties: + kind: + type: string name: type: string required: diff --git a/src/operator/otterizecrds/protectedservices-customresourcedefinition.yaml b/src/operator/otterizecrds/protectedservices-customresourcedefinition.yaml index aef1f8ba6..856532207 100644 --- a/src/operator/otterizecrds/protectedservices-customresourcedefinition.yaml +++ b/src/operator/otterizecrds/protectedservices-customresourcedefinition.yaml @@ -89,6 +89,8 @@ spec: spec: description: ProtectedServiceSpec defines the desired state of ProtectedService properties: + kind: + type: string name: type: string type: object diff --git a/src/operator/webhooks/clientintents_webhook_v1alpha3.go b/src/operator/webhooks/clientintents_webhook_v1alpha3.go index 3e2f9fe72..ef4439e2b 100644 --- a/src/operator/webhooks/clientintents_webhook_v1alpha3.go +++ b/src/operator/webhooks/clientintents_webhook_v1alpha3.go @@ -119,7 +119,7 @@ func (v *IntentsValidatorV1alpha3) validateNoDuplicateClients( desiredClientName := intentsObj.GetServiceName() for _, existingIntent := range intentsList.Items { // Deny admission if intents already exist for this client, and it's not the same object being updated - if existingIntent.GetServiceName() == desiredClientName && existingIntent.Name != intentsObj.Name { + if existingIntent.GetServiceName() == desiredClientName && existingIntent.Name != intentsObj.Name && existingIntent.GetClientKind() == intentsObj.GetClientKind() { return &field.Error{ Type: field.ErrorTypeDuplicate, Field: "name", @@ -134,6 +134,14 @@ func (v *IntentsValidatorV1alpha3) validateNoDuplicateClients( // validateSpec func (v *IntentsValidatorV1alpha3) validateSpec(intents *otterizev1alpha3.ClientIntents) *field.Error { + // validate that if kind is specified, it starts with an uppercase letter + if kind := intents.Spec.Service.Kind; kind != "" && strings.ToUpper(string(kind[0])) != string(kind[0]) { + return &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "kind", + Detail: "Kubernetes Kinds must start with an uppercase letter", + } + } for _, intent := range intents.GetCallsList() { if len(intent.Name) == 0 && intent.Type != otterizev1alpha3.IntentTypeInternet { return &field.Error{ diff --git a/src/shared/serviceidresolver/mocks/mock_service_resolver.go b/src/shared/serviceidresolver/mocks/mock_service_resolver.go index fd705947b..bdf423c5f 100644 --- a/src/shared/serviceidresolver/mocks/mock_service_resolver.go +++ b/src/shared/serviceidresolver/mocks/mock_service_resolver.go @@ -37,21 +37,6 @@ func (m *MockServiceResolver) EXPECT() *MockServiceResolverMockRecorder { return m.recorder } -// GetKubernetesServicesTargetingPod mocks base method. -func (m *MockServiceResolver) GetKubernetesServicesTargetingPod(ctx context.Context, pod *v1.Pod) ([]v1.Service, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetKubernetesServicesTargetingPod", ctx, pod) - ret0, _ := ret[0].([]v1.Service) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetKubernetesServicesTargetingPod indicates an expected call of GetKubernetesServicesTargetingPod. -func (mr *MockServiceResolverMockRecorder) GetKubernetesServicesTargetingPod(ctx, pod interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKubernetesServicesTargetingPod", reflect.TypeOf((*MockServiceResolver)(nil).GetKubernetesServicesTargetingPod), ctx, pod) -} - // ResolveClientIntentToPod mocks base method. func (m *MockServiceResolver) ResolveClientIntentToPod(ctx context.Context, intent v1alpha3.ClientIntents) (v1.Pod, error) { m.ctrl.T.Helper() @@ -67,21 +52,6 @@ func (mr *MockServiceResolverMockRecorder) ResolveClientIntentToPod(ctx, intent return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveClientIntentToPod", reflect.TypeOf((*MockServiceResolver)(nil).ResolveClientIntentToPod), ctx, intent) } -// ResolveIntentServerToPod mocks base method. -func (m *MockServiceResolver) ResolveIntentServerToPod(ctx context.Context, intent v1alpha3.Intent, namespace string) (v1.Pod, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ResolveIntentServerToPod", ctx, intent, namespace) - ret0, _ := ret[0].(v1.Pod) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ResolveIntentServerToPod indicates an expected call of ResolveIntentServerToPod. -func (mr *MockServiceResolverMockRecorder) ResolveIntentServerToPod(ctx, intent, namespace interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveIntentServerToPod", reflect.TypeOf((*MockServiceResolver)(nil).ResolveIntentServerToPod), ctx, intent, namespace) -} - // ResolvePodToServiceIdentity mocks base method. func (m *MockServiceResolver) ResolvePodToServiceIdentity(ctx context.Context, pod *v1.Pod) (serviceidentity.ServiceIdentity, error) { m.ctrl.T.Helper() @@ -96,3 +66,19 @@ func (mr *MockServiceResolverMockRecorder) ResolvePodToServiceIdentity(ctx, pod mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolvePodToServiceIdentity", reflect.TypeOf((*MockServiceResolver)(nil).ResolvePodToServiceIdentity), ctx, pod) } + +// ResolveServiceIdentityToPodSlice mocks base method. +func (m *MockServiceResolver) ResolveServiceIdentityToPodSlice(ctx context.Context, identity serviceidentity.ServiceIdentity) ([]v1.Pod, bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ResolveServiceIdentityToPodSlice", ctx, identity) + ret0, _ := ret[0].([]v1.Pod) + ret1, _ := ret[1].(bool) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// ResolveServiceIdentityToPodSlice indicates an expected call of ResolveServiceIdentityToPodSlice. +func (mr *MockServiceResolverMockRecorder) ResolveServiceIdentityToPodSlice(ctx, identity interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveServiceIdentityToPodSlice", reflect.TypeOf((*MockServiceResolver)(nil).ResolveServiceIdentityToPodSlice), ctx, identity) +} diff --git a/src/shared/serviceidresolver/serviceidentity/serviceidentity.go b/src/shared/serviceidresolver/serviceidentity/serviceidentity.go index 05203fff7..c75e77c2c 100644 --- a/src/shared/serviceidresolver/serviceidentity/serviceidentity.go +++ b/src/shared/serviceidresolver/serviceidentity/serviceidentity.go @@ -1,13 +1,19 @@ package serviceidentity import ( + "crypto/md5" + "encoding/hex" "fmt" - "github.com/otterize/intents-operator/src/operator/api/v1alpha3" "github.com/samber/lo" "sigs.k8s.io/controller-runtime/pkg/client" "strings" ) +const ( + MaxOtterizeNameLength = 20 + MaxNamespaceLength = 20 +) + type ServiceIdentity struct { Name string Namespace string @@ -17,12 +23,17 @@ type ServiceIdentity struct { } const KindService = "Service" +const KindOtterizeLegacy = "OttrLegacy" -func (si *ServiceIdentity) GetFormattedOtterizeIdentity() string { - if si.Kind == KindService { - return v1alpha3.GetFormattedOtterizeIdentity("svc."+si.Name, si.Namespace) +func (si *ServiceIdentity) GetFormattedOtterizeIdentityWithoutKind() string { + return getFormattedOtterizeIdentity(si.Name, si.Namespace) +} + +func (si *ServiceIdentity) GetFormattedOtterizeIdentityWithKind() string { + if si.Kind == "" || si.Kind == KindOtterizeLegacy { + return getFormattedOtterizeIdentity(si.Name, si.Namespace) } - return v1alpha3.GetFormattedOtterizeIdentity(si.Name, si.Namespace) + return getFormattedOtterizeIdentityWithKind(si.Name, si.Namespace, si.Kind) } func (si *ServiceIdentity) GetName() string { @@ -30,20 +41,56 @@ func (si *ServiceIdentity) GetName() string { } func (si *ServiceIdentity) GetNameWithKind() string { - return lo.Ternary(si.Kind == "", si.Name, fmt.Sprintf("%s-%s", si.Name, strings.ToLower(si.Kind))) + return lo.Ternary(si.Kind == "" || si.Kind == KindOtterizeLegacy, si.Name, fmt.Sprintf("%s-%s", si.Name, strings.ToLower(si.Kind))) } -func NewFromIntent(intent v1alpha3.Intent, clientNamespace string) ServiceIdentity { - return ServiceIdentity{ - Name: intent.GetTargetServerName(), - Namespace: intent.GetTargetServerNamespace(clientNamespace), - Kind: lo.Ternary(intent.IsTargetServerKubernetesService(), KindService, ""), +func (si *ServiceIdentity) String() string { + return fmt.Sprintf("%s/%s/%s", si.Kind, si.Namespace, si.Name) +} + +// getFormattedOtterizeIdentity truncates names and namespaces to a 20 char len string (if required) +// It also adds a short md5 hash of the full name+ns string and returns the formatted string +// This is due to Kubernetes' limit on 63 char label keys/values +func getFormattedOtterizeIdentity(name, ns string) string { + // Get MD5 for full length "name-namespace" string + hash := md5.Sum([]byte(fmt.Sprintf("%s-%s", name, ns))) + + // Truncate name and namespace to 20 chars each + if len(name) > MaxOtterizeNameLength { + name = name[:MaxOtterizeNameLength] + } + + if len(ns) > MaxNamespaceLength { + ns = ns[:MaxNamespaceLength] } + // A 6 char hash, even though truncated, leaves 2 ^ 48 combinations which should be enough + // for unique identities in a k8s cluster + hashSuffix := hex.EncodeToString(hash[:])[:6] + + return fmt.Sprintf("%s-%s-%s", name, ns, hashSuffix) + } -func NewFromClientIntent(clientIntent v1alpha3.ClientIntents) ServiceIdentity { - return ServiceIdentity{ - Name: clientIntent.Spec.Service.Name, - Namespace: clientIntent.Namespace, +func getFormattedOtterizeIdentityWithKind(name, ns, kind string) string { + // Get MD5 for full length "name-namespace" string + hash := md5.Sum([]byte(fmt.Sprintf("%s-%s-%s", name, ns, kind))) + + // Truncate name and namespace to 19 chars each + if len(name) > MaxOtterizeNameLength-1 { + name = name[:MaxOtterizeNameLength-1] + } + + if len(ns) > MaxNamespaceLength-1 { + ns = ns[:MaxNamespaceLength-1] } + + if len(kind) > 5 { + kind = kind[:5] + } + // A 6 char hash, even though truncated, leaves 2 ^ 48 combinations which should be enough + // for unique identities in a k8s cluster + hashSuffix := hex.EncodeToString(hash[:])[:6] + + return fmt.Sprintf("%s-%s-%s-%s", name, ns, strings.ToLower(kind), hashSuffix) + } diff --git a/src/shared/serviceidresolver/serviceidresolver.go b/src/shared/serviceidresolver/serviceidresolver.go index b247f9737..c84cbdff3 100644 --- a/src/shared/serviceidresolver/serviceidresolver.go +++ b/src/shared/serviceidresolver/serviceidresolver.go @@ -5,6 +5,7 @@ import ( "github.com/otterize/intents-operator/src/operator/api/v1alpha3" "github.com/otterize/intents-operator/src/shared/errors" "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" + "github.com/samber/lo" "github.com/sirupsen/logrus" "github.com/spf13/viper" corev1 "k8s.io/api/core/v1" @@ -22,9 +23,8 @@ var ErrPodNotFound = errors.NewSentinelError("pod not found") type ServiceResolver interface { ResolveClientIntentToPod(ctx context.Context, intent v1alpha3.ClientIntents) (corev1.Pod, error) - ResolveIntentServerToPod(ctx context.Context, intent v1alpha3.Intent, namespace string) (corev1.Pod, error) - GetKubernetesServicesTargetingPod(ctx context.Context, pod *corev1.Pod) ([]corev1.Service, error) ResolvePodToServiceIdentity(ctx context.Context, pod *corev1.Pod) (serviceidentity.ServiceIdentity, error) + ResolveServiceIdentityToPodSlice(ctx context.Context, identity serviceidentity.ServiceIdentity) ([]corev1.Pod, bool, error) } type Resolver struct { @@ -134,78 +134,33 @@ func (r *Resolver) GetOwnerObject(ctx context.Context, pod *corev1.Pod) (client. return obj, nil } -func (r *Resolver) ResolveClientIntentToPod(ctx context.Context, intent v1alpha3.ClientIntents) (corev1.Pod, error) { - podsList := &corev1.PodList{} - labelSelector, err := intent.BuildPodLabelSelector() +func (r *Resolver) ResolveServiceIdentityToPodSlice(ctx context.Context, identity serviceidentity.ServiceIdentity) ([]corev1.Pod, bool, error) { + labels, ok, err := v1alpha3.ServiceIdentityToLabelsForWorkloadSelection(ctx, r.client, identity) if err != nil { - return corev1.Pod{}, errors.Wrap(err) + return nil, false, errors.Wrap(err) } - err = r.client.List(ctx, podsList, client.MatchingLabelsSelector{Selector: labelSelector}) - if err != nil { - return corev1.Pod{}, errors.Wrap(err) - } - if len(podsList.Items) == 0 { - return corev1.Pod{}, errors.Wrap(ErrPodNotFound) + if !ok { + return nil, false, nil } - for _, pod := range podsList.Items { - if pod.DeletionTimestamp != nil { - continue - } - - return pod, nil + podList := &corev1.PodList{} + err = r.client.List(ctx, podList, &client.ListOptions{Namespace: identity.Namespace}, client.MatchingLabels(labels)) + if err != nil { + return nil, false, errors.Wrap(err) } + pods := lo.Filter(podList.Items, func(pod corev1.Pod, _ int) bool { return pod.DeletionTimestamp == nil }) - return corev1.Pod{}, errors.Wrap(ErrPodNotFound) + return pods, len(pods) > 0, nil } -func (r *Resolver) ResolveIntentServerToPod(ctx context.Context, intent v1alpha3.Intent, namespace string) (corev1.Pod, error) { - podsList := &corev1.PodList{} - - formattedTargetServer := v1alpha3.GetFormattedOtterizeIdentity(intent.GetTargetServerName(), namespace) - err := r.client.List( - ctx, - podsList, - client.MatchingLabels{v1alpha3.OtterizeServiceLabelKey: formattedTargetServer}, - client.InNamespace(namespace), - ) +func (r *Resolver) ResolveClientIntentToPod(ctx context.Context, intent v1alpha3.ClientIntents) (corev1.Pod, error) { + serviceID := intent.ToServiceIdentity() + pods, ok, err := r.ResolveServiceIdentityToPodSlice(ctx, serviceID) if err != nil { return corev1.Pod{}, errors.Wrap(err) } - if len(podsList.Items) == 0 { - return corev1.Pod{}, errors.Wrap(ErrPodNotFound) - } - - for _, pod := range podsList.Items { - if pod.DeletionTimestamp != nil { - continue - } - - return pod, nil - } - - return corev1.Pod{}, errors.Wrap(ErrPodNotFound) -} - -func (r *Resolver) GetKubernetesServicesTargetingPod(ctx context.Context, pod *corev1.Pod) ([]corev1.Service, error) { - serviceList := corev1.ServiceList{} - if err := r.client.List(ctx, &serviceList, &client.ListOptions{Namespace: pod.Namespace}); err != nil { - return nil, errors.Wrap(err) + if !ok { + return corev1.Pod{}, ErrPodNotFound } - - servicesTargetingPod := make([]corev1.Service, 0) - podLabels := pod.GetLabels() - - // Iterate over the services in the namespace, check their selector (which pods they are pointing to) - // and compare to the pod's labels. - for _, service := range serviceList.Items { - for podLabelKey, podLabelVal := range podLabels { - svcSelectorVal, ok := service.Spec.Selector[podLabelKey] - if ok && svcSelectorVal == podLabelVal { - servicesTargetingPod = append(servicesTargetingPod, service) - } - } - } - - return servicesTargetingPod, nil + return pods[0], nil } diff --git a/src/shared/serviceidresolver/serviceidresolver_test.go b/src/shared/serviceidresolver/serviceidresolver_test.go index 45e8a7bd8..2acb34dae 100644 --- a/src/shared/serviceidresolver/serviceidresolver_test.go +++ b/src/shared/serviceidresolver/serviceidresolver_test.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/otterize/intents-operator/src/operator/api/v1alpha3" serviceidresolvermocks "github.com/otterize/intents-operator/src/shared/serviceidresolver/mocks" + "github.com/otterize/intents-operator/src/shared/serviceidresolver/serviceidentity" "github.com/spf13/viper" "github.com/stretchr/testify/suite" "go.uber.org/mock/gomock" @@ -57,7 +58,7 @@ func (s *ServiceIdResolverTestSuite) TestResolveClientIntentToPod_PodExists() { SAName := "backendservice" intent := v1alpha3.ClientIntents{Spec: &v1alpha3.IntentsSpec{Service: v1alpha3.Service{Name: serviceName}}, ObjectMeta: metav1.ObjectMeta{Namespace: namespace}} - ls, err := intent.BuildPodLabelSelector() + ls, _, err := v1alpha3.ServiceIdentityToLabelsForWorkloadSelection(context.Background(), s.Client, intent.ToServiceIdentity()) s.Require().NoError(err) pod := corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, Spec: corev1.PodSpec{ServiceAccountName: SAName}} @@ -65,7 +66,8 @@ func (s *ServiceIdResolverTestSuite) TestResolveClientIntentToPod_PodExists() { s.Client.EXPECT().List( gomock.Any(), gomock.AssignableToTypeOf(&corev1.PodList{}), - &MatchingLabelsSelectorMatcher{client.MatchingLabelsSelector{Selector: ls}}, + gomock.Eq(&client.ListOptions{Namespace: namespace}), + gomock.Eq(ls), ).Do(func(_ any, podList *corev1.PodList, _ ...any) { podList.Items = append(podList.Items, pod) }) @@ -81,13 +83,14 @@ func (s *ServiceIdResolverTestSuite) TestResolveClientIntentToPod_PodDoesntExist namespace := "coolnamespace" intent := v1alpha3.ClientIntents{Spec: &v1alpha3.IntentsSpec{Service: v1alpha3.Service{Name: serviceName}}, ObjectMeta: metav1.ObjectMeta{Namespace: namespace}} - ls, err := intent.BuildPodLabelSelector() + ls, _, err := v1alpha3.ServiceIdentityToLabelsForWorkloadSelection(context.Background(), s.Client, intent.ToServiceIdentity()) s.Require().NoError(err) s.Client.EXPECT().List( gomock.Any(), gomock.AssignableToTypeOf(&corev1.PodList{}), - &MatchingLabelsSelectorMatcher{client.MatchingLabelsSelector{Selector: ls}}, + gomock.Eq(&client.ListOptions{Namespace: namespace}), + gomock.Eq(ls), ).Do(func(_ any, podList *corev1.PodList, _ ...any) {}) pod, err := s.Resolver.ResolveClientIntentToPod(context.Background(), intent) @@ -291,6 +294,81 @@ func (s *ServiceIdResolverTestSuite) TestJobWithNoParent() { s.Require().Equal(imageName, service.Name) } +func (s *ServiceIdResolverTestSuite) TestServiceIdentityToPodLabelsForWorkloadSelection_DeploymentKind() { + serviceName := "cool-service" + namespace := "cool-namespace" + service := serviceidentity.ServiceIdentity{Name: serviceName, Namespace: namespace, Kind: "Deployment"} + + s.Client.EXPECT().List(gomock.Any(), gomock.AssignableToTypeOf(&corev1.PodList{}), gomock.Eq(&client.ListOptions{Namespace: namespace}), map[string]string{ + v1alpha3.OtterizeServiceLabelKey: service.GetFormattedOtterizeIdentityWithoutKind(), + v1alpha3.OtterizeOwnerKindLabelKey: "Deployment", + }).Do(func(_ any, podList *corev1.PodList, _ ...any) { + podList.Items = append(podList.Items, corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "cool-pod", Namespace: namespace}}) + }) + + pods, ok, err := s.Resolver.ResolveServiceIdentityToPodSlice(context.Background(), service) + s.Require().NoError(err) + s.Require().True(ok) + s.Require().Len(pods, 1) +} + +func (s *ServiceIdResolverTestSuite) TestServiceIdentityToPodLabelsForWorkloadSelection_ServiceKind() { + serviceName := "cool-service" + namespace := "cool-namespace" + servicePodSelector := map[string]string{"cool-key": "cool-value"} + service := serviceidentity.ServiceIdentity{Name: serviceName, Namespace: namespace, Kind: serviceidentity.KindService} + + serviceObj := corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, + Spec: corev1.ServiceSpec{Selector: servicePodSelector}, + } + s.Client.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: serviceName, Namespace: namespace}, &corev1.Service{}).Do(func(_ context.Context, name types.NamespacedName, svc *corev1.Service, _ ...any) { + serviceObj.DeepCopyInto(svc) + }) + s.Client.EXPECT().List(gomock.Any(), gomock.AssignableToTypeOf(&corev1.PodList{}), gomock.Eq(&client.ListOptions{Namespace: namespace}), servicePodSelector).Do(func(_ any, podList *corev1.PodList, _ ...any) { + podList.Items = append(podList.Items, corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "cool-pod", Namespace: namespace}}) + }) + + pods, ok, err := s.Resolver.ResolveServiceIdentityToPodSlice(context.Background(), service) + s.Require().NoError(err) + s.Require().True(ok) + s.Require().Len(pods, 1) +} + +func (s *ServiceIdResolverTestSuite) TestServiceIdentityToPodLabelsForWorkloadSelection_ServiceKind_serviceNotFound() { + serviceName := "cool-service" + namespace := "cool-namespace" + service := serviceidentity.ServiceIdentity{Name: serviceName, Namespace: namespace, Kind: serviceidentity.KindService} + + s.Client.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: serviceName, Namespace: namespace}, &corev1.Service{}).Return(apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "services"}, serviceName)) + pods, ok, err := s.Resolver.ResolveServiceIdentityToPodSlice(context.Background(), service) + s.Require().NoError(err) + s.Require().False(ok) + s.Require().Len(pods, 0) +} + +func (s *ServiceIdResolverTestSuite) TestServiceIdentityToPodLabelsForWorkloadSelection_ServiceKind_podNotFound() { + serviceName := "cool-service" + namespace := "cool-namespace" + servicePodSelector := map[string]string{"cool-key": "cool-value"} + service := serviceidentity.ServiceIdentity{Name: serviceName, Namespace: namespace, Kind: serviceidentity.KindService} + + serviceObj := corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, + Spec: corev1.ServiceSpec{Selector: servicePodSelector}, + } + s.Client.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: serviceName, Namespace: namespace}, &corev1.Service{}).Do(func(_ context.Context, name types.NamespacedName, svc *corev1.Service, _ ...any) { + serviceObj.DeepCopyInto(svc) + }) + // empty list + s.Client.EXPECT().List(gomock.Any(), gomock.AssignableToTypeOf(&corev1.PodList{}), gomock.Eq(&client.ListOptions{Namespace: namespace}), servicePodSelector) + + pods, ok, err := s.Resolver.ResolveServiceIdentityToPodSlice(context.Background(), service) + s.Require().NoError(err) + s.Require().False(ok) + s.Require().Len(pods, 0) +} + func (s *ServiceIdResolverTestSuite) TestUserSpecifiedAnnotationForServiceName() { annotationName := "coolAnnotationName" expectedEnvVarName := "OTTERIZE_SERVICE_NAME_OVERRIDE_ANNOTATION" diff --git a/src/shared/testbase/testsuitebase.go b/src/shared/testbase/testsuitebase.go index 2e1209d35..0d3b14056 100644 --- a/src/shared/testbase/testsuitebase.go +++ b/src/shared/testbase/testsuitebase.go @@ -278,6 +278,8 @@ func (s *ControllerManagerTestSuiteBase) AddDeployment( UID: deployment.UID, }, } + err = s.Mgr.GetClient().Update(context.Background(), replicaSet) + s.Require().NoError(err) s.WaitUntilCondition(func(assert *assert.Assertions) { rs := &appsv1.ReplicaSet{} @@ -430,14 +432,16 @@ func (s *ControllerManagerTestSuiteBase) RemoveKafkaServerConfig(objName string) func (s *ControllerManagerTestSuiteBase) AddIntents( objName, - clientName string, + clientName, + clientKind string, callList []otterizev1alpha3.Intent) (*otterizev1alpha3.ClientIntents, error) { - return s.AddIntentsInNamespace(objName, clientName, s.TestNamespace, callList) + return s.AddIntentsInNamespace(objName, clientName, clientKind, s.TestNamespace, callList) } func (s *ControllerManagerTestSuiteBase) AddIntentsInNamespace( objName, clientName string, + clientKind string, namespace string, callList []otterizev1alpha3.Intent) (*otterizev1alpha3.ClientIntents, error) { @@ -451,7 +455,7 @@ func (s *ControllerManagerTestSuiteBase) AddIntentsInNamespace( }, }, Spec: &otterizev1alpha3.IntentsSpec{ - Service: otterizev1alpha3.Service{Name: clientName}, + Service: otterizev1alpha3.Service{Name: clientName, Kind: clientKind}, Calls: callList, }, }