Add new NonClusterHost resource for log ingestion

[hjiawei]

Description

This change introduces a new NonClusterHost custom resource for Calico Enterprise to support non-cluster hosts log ingestion.

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.

[dimitri-nicolo]

This seems like a required field. Should we add:
json:"endpoint" validate:"required"

[dimitri-nicolo]

It might be worth writing a custom validator, so that the string input that does not match the a predefined regex pattern prints an error at the time of creation.

This might work: // +kubebuilder:validation:Pattern=^https?://.+$``

[hjiawei]

Validation pattern is added to the endpoint field.

[rene-dekker]

Since NonClusterHostSpec is not a required field, please mind that it is still possible to create this resource without the Endpoint field. Does that have any implications for the feature? Should the controller still validate that the field exists?

[caseydavenport]

NonClusterHostSpec isn't required, but it's also not a pointer, which means it will be present regardless and so this field should always be defaulted by the kubebuilder validation IIUC

[rene-dekker]

I applied the CRD to my kind cluster:

$ k apply -f nonclust.yaml 
The NonClusterHost "default" is invalid: spec.endpoint: Required value

 1   12:17:55   ~/dev/kind    
$ cat nonclust.yaml 
kind: NonClusterHost
apiVersion: operator.tigera.io/v1
metadata:
  name: default
spec: {}

But then:

$ k apply -f nonclust.yaml 
nonclusterhost.operator.tigera.io/default configured
 0   12:18:26   ~/dev/kind    
$ cat nonclust.yaml 
kind: NonClusterHost
apiVersion: operator.tigera.io/v1
metadata:
  name: default
spec: null

[caseydavenport]

hmm, guess I was wrong about that. Surprised that spec: null is valid at all considering the spec isn't a pointer.

[rene-dekker]

It is identical to this:

kind: NonClusterHost
apiVersion: operator.tigera.io/v1
metadata:
  name: default

[rene-dekker]

I was wondering if there is any harm in removing this if-statement.

[hjiawei]

If a nonclusterhost resource is accidentally applied to a management cluster, this check prevents opening the ingestion endpoint.

[rene-dekker]

I meant to say that I don't really see the problem in having a management cluster that also has non cluster hosts working at the same time.

[caseydavenport]

At a minimum, I would prefer if we inverted this check - i.e., "If both are specified, error" instead of silently ignoring an unsupported configuration.

I'm fine with erroring for now if MCM is out of scope, but also fine to allow it in management clusters if we think it will come relatively for free.

[hjiawei]

spec.endpoint is validated in the manager controller. When NonClusterHost resource is applied to the cluster and the endpoint filed is an invalid URL, manager controller will be degraded.

The managementCluster == nil check is also removed because management cluster should support this feature.

[rene-dekker]

lgtm