Adjust connection controller and how it handles the license and network policy

[tmjd]

Description

For PR author

• Tests for change.
• If changing pkg/apis/, run make gen-files
• If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

• Milestone set according to targeted release.
• Appropriate labels:
  • kind/bug if this is a bugfix.
  • kind/enhancement if this is a a new feature.
  • enterprise if this PR applies to Calico Enterprise only.

[pasanw]

The current implementation gates whether or not we try to render network policy based on whether the tier is present. I'm wondering if we should just change this so that it gates based on whether the license is present.

I think this achieves the same result as what you've suggested, but the code would read a bit simpler. We would switch the tier check and the license check, e.g.

	includeV3NetworkPolicy := false
	if license, err := utils.FetchLicenseKey(ctx, r.Client); err != nil {
		if !k8serrors.IsNotFound(err) {
                   // error
		}	
	} else {
		includeV3NetworkPolicy = true


		if err := r.Client.Get(ctx, client.ObjectKey{Name: networkpolicy.TigeraComponentTierName}, &v3.Tier{}); err != nil {
			// error
		}

		if egressAccessControlRequired(managementClusterConnection.Spec.ManagementClusterAddr, r.resolvedPodProxies, log) {
			if !utils.IsFeatureActive(license, common.EgressAccessControlFeature) {
				// error
			}
		}
	}

[tmjd]

I don't think what you've suggested is sufficient.
We need to allow rendering in this controller as much as we can meaning if we can't add the policy then we leave it out if the tier or the license is missing.

	includeV3NetworkPolicy := false
	if license, err := utils.FetchLicenseKey(ctx, r.Client); err != nil {
		if !k8serrors.IsNotFound(err) {
			// error
		}	
	} else {
		if err := r.Client.Get(ctx, client.ObjectKey{Name: networkpolicy.TigeraComponentTierName}, &v3.Tier{}); err == nil {
			if egressAccessControlRequired(managementClusterConnection.Spec.ManagementClusterAddr, r.resolvedPodProxies, log) {
				if !utils.IsFeatureActive(license, common.EgressAccessControlFeature) {
					// error
				}
			}
			includeV3NetworkPolicy = true
		} else {
			if !k8serrors.IsNotFound(err) {
				// error because getting the tier failed 
			}
			// don't error if Tier is missing so we can still render but without the policy
		}
	}

If we want to simplify the code, I'm wondering, do we actually need the egressAccessControlRequired check? Couldn't we always assume that we need EgressAccessControlFeature? Do we have any expectations that we'll have configurations where ManagedCluster functionality is used but EgressAccess control is not enabled?

[pasanw]

The only time it's not required is if the management cluster host is a domain rather than an IP. I want to say that we should not expect to see a user who uses an IP for their mgmt cluster host and also has a license that does not include EgressAccessControl... but I can't guarantee it?

Nitpicking here, but if we want to proceed without network policy if either license or tier is not found, maybe it could be structured like this?

	license, err := utils.FetchLicenseKey(ctx, r.Client)
	if err != nil {
		if !k8serrors.IsNotFound(err) {
			// error
		}
	}
	licenseFound := err != nil
	
	if err := r.Client.Get(ctx, client.ObjectKey{Name: networkpolicy.TigeraComponentTierName}, &v3.Tier{}); err != nil {
		if !k8serrors.IsNotFound(err) {
			// error
		}
	}
	tierFound := err != nil
	
	includeV3NetworkPolicy := false
	if licenseFound && tierFound {
		// check license feature
		
		includeV3NetworkPolicy = true
	}

[tmjd]

I've asked about licenses with limited feature sets and it sounds like we've never given anything like that out to customers, so I think we can safely not worry about the edge case of a managed cluster without the EgressAccessControl feature.