Skip to content

Add Calico Goldmane flow collector support to Operator#337

Open
pavankumarinnamuri wants to merge 8 commits intomainfrom
calicogoldmane
Open

Add Calico Goldmane flow collector support to Operator#337
pavankumarinnamuri wants to merge 8 commits intomainfrom
calicogoldmane

Conversation

@pavankumarinnamuri
Copy link
Contributor

  • Add Goldmane proto definitions and generated gRPC code
  • Implement Calico flow collector for Goldmane integration
  • Add RBAC role and rolebinding for Goldmane access in Calico namespace
  • Update values.schema.json for new configuration options

- Add Goldmane proto definitions and generated gRPC code
- Implement Calico flow collector for Goldmane integration
- Add RBAC role and rolebinding for Goldmane access in Calico namespace
- Update values.schema.json for new configuration options
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for collecting network flows from Calico's Goldmane service, expanding the operator's CNI compatibility beyond Cilium to include Calico. The implementation includes protobuf definitions for the Goldmane gRPC API, a flow collector that connects to Goldmane via mTLS, integration into the existing stream management architecture, and RBAC resources for accessing Goldmane services and secrets in the Calico namespace.

Changes:

  • Add Goldmane protobuf definitions and generated gRPC client code for flow streaming
  • Implement CalicoFlowCollector with Goldmane service discovery, mTLS connection, and flow conversion logic
  • Integrate Calico flow collector into stream management with priority-based CNI detection (Cilium > Calico > OVN-K > Falco)
  • Add Kubernetes RBAC Role and RoleBinding for accessing Goldmane service and secrets in the Calico namespace
  • Update Helm values schema to include calico.namespace configuration option

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
api/illumio/cloud/goldmane/v1/goldmane.proto Defines protobuf schema for Goldmane flow streaming API
api/illumio/cloud/goldmane/v1/goldmane.pb.go Generated Go protobuf code for Goldmane messages
api/illumio/cloud/goldmane/v1/goldmane_grpc.pb.go Generated Go gRPC client/server code for Goldmane service
internal/controller/goldmane/goldmane.go Implements Goldmane service discovery, mTLS configuration, and gRPC connection
internal/controller/calico_flow_collector.go Implements CalicoFlowCollector that streams and converts Goldmane flows
internal/controller/streams.go Integrates Calico flow collector into CNI detection and streaming logic
internal/controller/streams_helper.go Adds CalicoFlow case to network flow request handler
cloud-operator/templates/role.yaml Adds RBAC Role for reading Goldmane service and secrets
cloud-operator/templates/rolebinding.yaml Adds RoleBinding to grant operator access to Goldmane resources
cloud-operator/values.schema.json Adds calico.namespace configuration property
Makefile Updates Docker username and image tag (appears to be development changes)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +46 to +53
{{- if (lookup "v1" "Namespace" "" .Values.calico.namespace) }}
---
# RoleBinding for Goldmane access in Calico namespace
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: cloud-operator-goldmane-reader
namespace: {{ .Values.calico.namespace }}
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The template uses .Values.calico.namespace without checking if it's empty or nil. If calico.namespace is not set in values, this will cause issues. The lookup function may prevent template rendering errors, but .Values.calico.namespace should be checked for emptiness before use (e.g., using 'if .Values.calico.namespace' or providing a default value).

Copilot uses AI. Check for mistakes.
Comment on lines +196 to +208
// parseUnixTimestamp parses a Unix timestamp string to a protobuf Timestamp.
func parseUnixTimestamp(s string) *timestamppb.Timestamp {
if s == "" {
return nil
}

seconds, err := strconv.ParseInt(s, 10, 64)
if err != nil {
return nil
}

return &timestamppb.Timestamp{Seconds: seconds}
}
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing errors are silently ignored by returning nil or 0. This could mask data corruption or format issues from Goldmane. Consider logging warnings when parsing fails so that administrators are aware of malformed data being received from the Goldmane service.

Copilot uses AI. Check for mistakes.
# Variables
APP_NAME := operator
DOCKER_USERNAME := aaronnguyenillumio
DOCKER_USERNAME := pki619
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DOCKER_USERNAME has been changed from 'aaronnguyenillumio' to 'pki619'. This appears to be a personal username change that should not be committed to the repository. This should either be reverted or configured through an environment variable to avoid hardcoding personal usernames in the Makefile.

Suggested change
DOCKER_USERNAME := pki619
# DOCKER_USERNAME is expected to be provided via the environment,
# for example: export DOCKER_USERNAME=your-dockerhub-username

Copilot uses AI. Check for mistakes.
docker-build:
@echo "Building Docker image..."
docker buildx build --platform linux/amd64,linux/arm64 --load -t $(DOCKER_IMAGE):latest .
docker buildx build --platform linux/amd64,linux/arm64 --load -t $(DOCKER_IMAGE):calicof .
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Docker image tag has been changed from 'latest' to 'calicof', which appears to be a development or testing tag. This should not be committed to the main branch as it changes the default image tag for all users. Consider reverting this to 'latest' or using a proper versioning scheme.

Copilot uses AI. Check for mistakes.
Comment on lines +210 to +222
// parseUint32 parses a string to uint32.
func parseUint32(s string) uint32 {
if s == "" {
return 0
}

val, err := strconv.ParseUint(s, 10, 32)
if err != nil {
return 0
}

return uint32(val)
}
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing errors are silently ignored by returning 0. This could mask data corruption or format issues from Goldmane. Consider logging warnings when parsing fails so that administrators are aware of malformed data being received from the Goldmane service.

Copilot uses AI. Check for mistakes.
docker-push:
@echo "Pushing Docker image to Docker Hub..."
docker push $(DOCKER_IMAGE):latest
docker push $(DOCKER_IMAGE):calicof
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Docker image tag has been changed from 'latest' to 'calicof', which appears to be a development or testing tag. This should not be committed to the main branch as it changes the default image tag for all users. Consider reverting this to 'latest' or using a proper versioning scheme.

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +46
{{- if (lookup "v1" "Namespace" "" .Values.calico.namespace) }}
---
# Role to read Goldmane service and secret in Calico namespace
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: goldmane-reader
namespace: {{ .Values.calico.namespace }}
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The template uses .Values.calico.namespace without checking if it's empty or nil. If calico.namespace is not set in values, this will cause issues. The lookup function may prevent template rendering errors, but .Values.calico.namespace should be checked for emptiness before use (e.g., using 'if .Values.calico.namespace' or providing a default value).

Suggested change
{{- if (lookup "v1" "Namespace" "" .Values.calico.namespace) }}
---
# Role to read Goldmane service and secret in Calico namespace
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: goldmane-reader
namespace: {{ .Values.calico.namespace }}
{{- $calicoNs := default "" .Values.calico.namespace }}
{{- if and $calicoNs (lookup "v1" "Namespace" "" $calicoNs) }}
---
# Role to read Goldmane service and secret in Calico namespace
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: goldmane-reader
namespace: {{ $calicoNs }}

Copilot uses AI. Check for mistakes.
return calicoFlow
}

// convertGoldmanePolicies converts Goldmane Policies to CalicoPolices.
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment contains a typo: 'CalicoPolices' should be 'CalicoPolicies'.

Suggested change
// convertGoldmanePolicies converts Goldmane Policies to CalicoPolices.
// convertGoldmanePolicies converts Goldmane Policies to CalicoPolicies.

Copilot uses AI. Check for mistakes.
"type": "object",
"properties": {
"namespace": {
"type": "string"
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The calico.namespace property in the schema does not specify whether it's required or provide a default value. Consider adding validation constraints such as 'required', 'minLength', or a 'default' value to ensure the field is properly configured when Calico integration is intended to be used.

Suggested change
"type": "string"
"type": "string",
"minLength": 1

Copilot uses AI. Check for mistakes.
@illumio illumio deleted a comment from Copilot AI Jan 19, 2026
@illumio illumio deleted a comment from Copilot AI Jan 19, 2026
@illumio illumio deleted a comment from Copilot AI Jan 19, 2026
@pavankumarinnamuri pavankumarinnamuri marked this pull request as ready for review January 19, 2026 23:26
@pavankumarinnamuri pavankumarinnamuri changed the title Add Calico Goldmane flow collector support with RBAC Add Calico Goldmane flow collector support to Operator Jan 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants