Fixed bug where unresolved DNS names would cause egress network polic…

…ies to not be created (#436)

src/operator/controllers/intents_reconcilers/consts/consts.go

@@ -2,31 +2,31 @@ package consts
 
 // Consts have to go here to prevent import cycle between istiopolicy and intents_reconcilers.
 const (
-	ReasonEnforcementDefaultOff                      = "EnforcementGloballyDisabled"
-	ReasonNetworkPolicyCreationDisabled              = "NetworkPolicyCreationDisabled"
-	ReasonGettingNetworkPolicyFailed                 = "GettingNetworkPolicyFailed"
-	ReasonRemovingNetworkPolicyFailed                = "RemovingNetworkPolicyFailed"
-	ReasonReconcilingNetworkPolicyFailed             = "ReconcilingNetworkPolicyFailed"
-	ReasonNamespaceNotAllowed                        = "NamespaceNotAllowed"
-	ReasonCreatingNetworkPoliciesFailed              = "CreatingNetworkPoliciesFailed"
-	ReasonCreatedNetworkPolicies                     = "CreatedNetworkPolicies"
-	ReasonIstioPolicyCreationDisabled                = "IstioPolicyCreationDisabled"
-	ReasonRemovingIstioPolicyFailed                  = "RemovingIstioPolicyFailed"
-	ReasonPodsNotFound                               = "PodsNotFound"
-	ReasonIntentsFoundButNoServiceAccount            = "ReasonIntentsFoundButNoServiceAccount"
-	ReasonReconciledAWSPolicies                      = "ReasonReconciledAWSPolicies"
-	ReasonReconcilingAWSPoliciesFailed               = "ReasonReconcilingAWSPoliciesFailed"
-	ReasonReconciledIAMPolicies                      = "ReasonReconciledIAMPolicies"
-	ReasonReconcilingIAMPoliciesFailed               = "ReasonReconcilingIAMPoliciesFailed"
-	ReasonIntentsServiceAccountUsedByMultipleClients = "ReasonIntentsServiceAccountUsedByMultipleClients"
-	ReasonKubernetesServiceNotFound                  = "KubernetesServiceNotFound"
-	ReasonPortRestrictionUnsupportedForStrings       = "TypeStringPortNotSupported"
-	ReasonEgressNetworkPolicyCreationDisabled        = "EgressNetworkPolicyCreationDisabled"
-	ReasonGettingEgressNetworkPolicyFailed           = "GettingEgressNetworkPolicyFailed"
-	ReasonRemovingEgressNetworkPolicyFailed          = "RemovingEgressNetworkPolicyFailed"
-	ReasonCreatingEgressNetworkPoliciesFailed        = "CreatingEgressNetworkPoliciesFailed"
-	ReasonCreatedEgressNetworkPolicies               = "CreatedEgressNetworkPolicies"
-	ReasonCreatedInternetEgressNetworkPolicies       = "CreatedInternetEgressNetworkPolicies"
-	ReasonIntentToUnresolvedDns                      = "IntentToUnresolvedDns"
-	ReasonNetworkPolicyCreationFailedMissingIP       = "NetworkPolicyCreationFailedMissingIP"
+	ReasonEnforcementDefaultOff                                   = "EnforcementGloballyDisabled"
+	ReasonNetworkPolicyCreationDisabled                           = "NetworkPolicyCreationDisabled"
+	ReasonGettingNetworkPolicyFailed                              = "GettingNetworkPolicyFailed"
+	ReasonRemovingNetworkPolicyFailed                             = "RemovingNetworkPolicyFailed"
+	ReasonReconcilingNetworkPolicyFailed                          = "ReconcilingNetworkPolicyFailed"
+	ReasonNamespaceNotAllowed                                     = "NamespaceNotAllowed"
+	ReasonCreatingNetworkPoliciesFailed                           = "CreatingNetworkPoliciesFailed"
+	ReasonCreatedNetworkPolicies                                  = "CreatedNetworkPolicies"
+	ReasonIstioPolicyCreationDisabled                             = "IstioPolicyCreationDisabled"
+	ReasonRemovingIstioPolicyFailed                               = "RemovingIstioPolicyFailed"
+	ReasonPodsNotFound                                            = "PodsNotFound"
+	ReasonIntentsFoundButNoServiceAccount                         = "ReasonIntentsFoundButNoServiceAccount"
+	ReasonReconciledAWSPolicies                                   = "ReasonReconciledAWSPolicies"
+	ReasonReconcilingAWSPoliciesFailed                            = "ReasonReconcilingAWSPoliciesFailed"
+	ReasonReconciledIAMPolicies                                   = "ReasonReconciledIAMPolicies"
+	ReasonReconcilingIAMPoliciesFailed                            = "ReasonReconcilingIAMPoliciesFailed"
+	ReasonIntentsServiceAccountUsedByMultipleClients              = "ReasonIntentsServiceAccountUsedByMultipleClients"
+	ReasonKubernetesServiceNotFound                               = "KubernetesServiceNotFound"
+	ReasonPortRestrictionUnsupportedForStrings                    = "TypeStringPortNotSupported"
+	ReasonEgressNetworkPolicyCreationDisabled                     = "EgressNetworkPolicyCreationDisabled"
+	ReasonGettingEgressNetworkPolicyFailed                        = "GettingEgressNetworkPolicyFailed"
+	ReasonRemovingEgressNetworkPolicyFailed                       = "RemovingEgressNetworkPolicyFailed"
+	ReasonCreatingEgressNetworkPoliciesFailed                     = "CreatingEgressNetworkPoliciesFailed"
+	ReasonCreatedEgressNetworkPolicies                            = "CreatedEgressNetworkPolicies"
+	ReasonCreatedInternetEgressNetworkPolicies                    = "CreatedInternetEgressNetworkPolicies"
+	ReasonIntentToUnresolvedDns                                   = "IntentToUnresolvedDns"
+	ReasonInternetEgressNetworkPolicyCreationWaitingUnresolvedDNS = "InternetEgressNetworkPolicyCreationWaitingUnresolvedDNS"
 )

src/operator/controllers/intents_reconcilers/networkpolicy/builders/internet_network_policy.go

@@ -37,21 +37,20 @@ func (r *InternetEgressRulesBuilder) buildEgressRules(ep effectivepolicy.Service
 	}
 
 	for _, intent := range intents {
-		peers, ports, ok, err := r.buildRuleForIntent(intent, ep)
+		peers, ports, foundResolvedDNSNames, err := r.buildRuleForIntent(intent, ep)
 		if err != nil {
 			return nil, errors.Wrap(err)
 		}
-		if !ok {
+		if !foundResolvedDNSNames {
+			ep.ClientIntentsEventRecorder.RecordNormalEventf(consts.ReasonInternetEgressNetworkPolicyCreationWaitingUnresolvedDNS, "Network policy not created for internet intents as no DNS names were resolved yet; once traffic is observed, a matching network policy will be created")
 			continue
 		}
 		rules = append(rules, v1.NetworkPolicyEgressRule{
 			To:    peers,
 			Ports: ports,
 		})
 	}
-	if len(rules) == 0 {
-		return nil, errors.New("cannot create rules for internet network policy")
-	}
+
 	return rules, nil
 }
 
@@ -62,11 +61,6 @@ func (r *InternetEgressRulesBuilder) buildRuleForIntent(intent otterizev1alpha3.
 	ips = append(ips, intent.Internet.Ips...)
 
 	if len(ips) == 0 {
-		dnsNames := lo.Reduce(intent.Internet.Domains, func(names, dns string, _ int) string {
-			return fmt.Sprintf("%s %s", names, dns)
-		}, "")
-
-		ep.ClientIntentsEventRecorder.RecordWarningEventf(consts.ReasonNetworkPolicyCreationFailedMissingIP, "no IPs found for internet intent %s", dnsNames)
 		return nil, nil, false, nil
 	}
 
@@ -87,7 +81,6 @@ func (r *InternetEgressRulesBuilder) getIpsForDNS(intent otterizev1alpha3.Intent
 		})
 
 		if !found {
-			ep.ClientIntentsEventRecorder.RecordWarningEventf(consts.ReasonIntentToUnresolvedDns, "could not find IP for DNS %s", dns)
 			continue
 		}
 

src/operator/controllers/intents_reconcilers/networkpolicy/builders/internet_network_policy_test.go

@@ -133,8 +133,8 @@ func (s *InternetNetworkPolicyReconcilerTestSuite) TestCreateNetworkPolicySingle
 	s.ignoreRemoveOrphan()
 
 	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
-	s.NoError(err)
-	s.Empty(res)
+	s.Require().NoError(err)
+	s.Require().Empty(res)
 	s.ExpectEvent(consts.ReasonCreatedEgressNetworkPolicies)
 }
 
@@ -240,8 +240,8 @@ func (s *InternetNetworkPolicyReconcilerTestSuite) TestCreateNetworkPolicyForDNS
 	s.ignoreRemoveOrphan()
 
 	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
-	s.NoError(err)
-	s.Empty(res)
+	s.Require().NoError(err)
+	s.Require().Empty(res)
 	s.ExpectEvent(consts.ReasonCreatedEgressNetworkPolicies)
 }
 
@@ -345,8 +345,8 @@ func (s *InternetNetworkPolicyReconcilerTestSuite) TestCreateNetworkPolicyFromDN
 	s.ignoreRemoveOrphan()
 
 	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
-	s.NoError(err)
-	s.Empty(res)
+	s.Require().NoError(err)
+	s.Require().Empty(res)
 	s.ExpectEvent(consts.ReasonCreatedEgressNetworkPolicies)
 }
 
@@ -474,8 +474,8 @@ func (s *InternetNetworkPolicyReconcilerTestSuite) TestCreateNetworkPolicyMultip
 	s.ignoreRemoveOrphan()
 
 	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
-	s.NoError(err)
-	s.Empty(res)
+	s.Require().NoError(err)
+	s.Require().Empty(res)
 	s.ExpectEvent(consts.ReasonCreatedEgressNetworkPolicies)
 }
 
@@ -565,8 +565,8 @@ func (s *InternetNetworkPolicyReconcilerTestSuite) TestNetworkPolicyDeletedClean
 	s.Client.EXPECT().Delete(gomock.Any(), gomock.Eq(existingPolicy)).Return(nil)
 
 	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
-	s.NoError(err)
-	s.Empty(res)
+	s.Require().NoError(err)
+	s.Require().Empty(res)
 }
 
 func (s *InternetNetworkPolicyReconcilerTestSuite) TestUpdateNetworkPolicy() {
@@ -672,8 +672,8 @@ func (s *InternetNetworkPolicyReconcilerTestSuite) TestUpdateNetworkPolicy() {
 	s.Client.EXPECT().Patch(gomock.Any(), gomock.Eq(newPolicy), intents_reconcilers.MatchPatch(client.MergeFrom(existingBadPolicy))).Return(nil)
 
 	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
-	s.NoError(err)
-	s.Empty(res)
+	s.Require().NoError(err)
+	s.Require().Empty(res)
 	s.ExpectEvent(consts.ReasonCreatedEgressNetworkPolicies)
 }
 
@@ -826,7 +826,7 @@ func (s *InternetNetworkPolicyReconcilerTestSuite) TestRemoveOrphanNetworkPolicy
 		})
 
 	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
-	s.NoError(err)
+	s.Require().NoError(err)
 	s.Empty(res)
 }
 
@@ -882,11 +882,11 @@ func (s *InternetNetworkPolicyReconcilerTestSuite) testEnforcementDisabled() {
 	s.ignoreRemoveOrphan()
 
 	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
-	s.NoError(err)
+	s.Require().NoError(err)
 	s.Empty(res)
 }
 
-func (s *InternetNetworkPolicyReconcilerTestSuite) TestNoIpFoundForDNS() {
+func (s *InternetNetworkPolicyReconcilerTestSuite) TestNoIpFoundForOneDNSButFoundForAnotherGeneratesRuleSuccessfully() {
 	s.Reconciler.EnforcementDefaultState = true
 
 	clientIntentsName := "client-intents"
@@ -980,9 +980,8 @@ func (s *InternetNetworkPolicyReconcilerTestSuite) TestNoIpFoundForDNS() {
 	s.externalNetpolHandler.EXPECT().HandlePodsByLabelSelector(gomock.Any(), gomock.Any(), gomock.Any())
 
 	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
-	s.NoError(err)
-	s.Empty(res)
-	s.ExpectEvent(consts.ReasonIntentToUnresolvedDns)
+	s.Require().NoError(err)
+	s.Require().Empty(res)
 	s.ExpectEvent(consts.ReasonCreatedEgressNetworkPolicies)
 }
 
@@ -1024,14 +1023,12 @@ func (s *InternetNetworkPolicyReconcilerTestSuite) TestNoIpFoundForAnyDNS() {
 	clientIntents.Namespace = clientNamespace
 	clientIntents.Name = clientIntentsName
 	s.expectGetAllEffectivePolicies([]otterizev1alpha3.ClientIntents{clientIntents})
+	s.ignoreRemoveOrphan()
 
 	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
-	s.Error(err, "cannot create rules for internet network policy")
-	s.Empty(res)
-	s.ExpectEvent(consts.ReasonIntentToUnresolvedDns)
-	s.ExpectEvent(consts.ReasonIntentToUnresolvedDns)
-	s.ExpectEvent(consts.ReasonNetworkPolicyCreationFailedMissingIP)
-	s.ExpectEvent(consts.ReasonCreatingEgressNetworkPoliciesFailed)
+	s.Require().NoError(err)
+	s.Require().Empty(res)
+	s.ExpectEvent(consts.ReasonInternetEgressNetworkPolicyCreationWaitingUnresolvedDNS)
 }
 
 func TestInternetNetworkPolicyReconcilerTestSuite(t *testing.T) {

src/shared/testbase/mocks_tests_suite_base.go

@@ -38,7 +38,7 @@ func (s *MocksSuiteBase) ExpectEvent(expectedEventReason string) {
 	case event := <-s.Recorder.Events:
 		s.Require().Contains(event, expectedEventReason)
 	default:
-		s.Fail("Expected event not found")
+		s.Failf("Expected event not found", "Event '%v'", expectedEventReason)
 	}
 }