Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NETOBSERV-963, revert most of cert watching [1.2 backport] #315

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 3 additions & 6 deletions controllers/consoleplugin/consoleplugin_objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
flowslatest "github.com/netobserv/network-observability-operator/api/v1beta1"
"github.com/netobserv/network-observability-operator/controllers/constants"
"github.com/netobserv/network-observability-operator/pkg/helper"
"github.com/netobserv/network-observability-operator/pkg/watchers"
)

const secretName = "console-serving-cert"
Expand All @@ -39,10 +38,9 @@ type builder struct {
selector map[string]string
desired *flowslatest.FlowCollectorSpec
imageName string
cWatcher *watchers.CertificatesWatcher
}

func newBuilder(ns, imageName string, desired *flowslatest.FlowCollectorSpec, cWatcher *watchers.CertificatesWatcher) builder {
func newBuilder(ns, imageName string, desired *flowslatest.FlowCollectorSpec) builder {
version := helper.ExtractVersion(imageName)
return builder{
namespace: ns,
Expand All @@ -55,7 +53,6 @@ func newBuilder(ns, imageName string, desired *flowslatest.FlowCollectorSpec, cW
},
desired: desired,
imageName: imageName,
cWatcher: cWatcher,
}
}

Expand Down Expand Up @@ -236,12 +233,12 @@ func (b *builder) podTemplate(cmDigest string) *corev1.PodTemplateSpec {

args := buildArgs(b.desired)
if b.desired != nil && b.desired.Loki.TLS.Enable && !b.desired.Loki.TLS.InsecureSkipVerify {
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &b.desired.Loki.TLS, lokiCerts, b.cWatcher)
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &b.desired.Loki.TLS, lokiCerts)
}

statusTLS := helper.GetLokiStatusTLS(&b.desired.Loki)
if b.desired != nil && statusTLS.Enable && !statusTLS.InsecureSkipVerify {
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &statusTLS, lokiStatusCerts, b.cWatcher)
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &statusTLS, lokiStatusCerts)
}

if helper.LokiUseHostToken(&b.desired.Loki) {
Expand Down
6 changes: 1 addition & 5 deletions controllers/consoleplugin/consoleplugin_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (r *CPReconciler) Reconcile(ctx context.Context, desired *flowslatest.FlowC
}

// Create object builder
builder := newBuilder(ns, r.image, &desired.Spec, r.CertWatcher)
builder := newBuilder(ns, r.image, &desired.Spec)

if err := r.reconcilePermissions(ctx, &builder); err != nil {
return err
Expand Down Expand Up @@ -196,10 +196,6 @@ func (r *CPReconciler) reconcileDeployment(ctx context.Context, builder builder,
defer report.LogIfNeeded(ctx)

newDepl := builder.deployment(cmDigest)
// Annotate pod with certificate reference so that it is reloaded if modified
if err := r.CertWatcher.AnnotatePod(ctx, r.Client, &newDepl.Spec.Template, lokiCerts, lokiStatusCerts); err != nil {
return err
}
if !r.nobjMngr.Exists(r.owned.deployment) {
if err := r.CreateOwned(ctx, newDepl); err != nil {
return err
Expand Down
20 changes: 9 additions & 11 deletions controllers/consoleplugin/consoleplugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
flowslatest "github.com/netobserv/network-observability-operator/api/v1beta1"
"github.com/netobserv/network-observability-operator/controllers/constants"
"github.com/netobserv/network-observability-operator/pkg/helper"
"github.com/netobserv/network-observability-operator/pkg/watchers"

promConfig "github.com/prometheus/common/config"
)
Expand All @@ -28,7 +27,6 @@ var testResources = corev1.ResourceRequirements{
corev1.ResourceMemory: resource.MustParse("512Mi"),
},
}
var certWatcher = watchers.NewCertificatesWatcher()

func getPluginConfig() flowslatest.FlowCollectorConsolePlugin {
return flowslatest.FlowCollectorConsolePlugin{
Expand Down Expand Up @@ -111,7 +109,7 @@ func TestContainerUpdateCheck(t *testing.T) {
plugin := getPluginConfig()
loki := flowslatest.FlowCollectorLoki{URL: "http://loki:3100/", TenantID: "netobserv"}
spec := flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder := newBuilder(testNamespace, testImage, &spec, &certWatcher)
builder := newBuilder(testNamespace, testImage, &spec)
old := builder.deployment("digest")
new := builder.deployment("digest")
report := helper.NewChangeReport("")
Expand Down Expand Up @@ -163,7 +161,7 @@ func TestContainerUpdateCheck(t *testing.T) {
},
}}
spec = flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder = newBuilder(testNamespace, testImage, &spec, &certWatcher)
builder = newBuilder(testNamespace, testImage, &spec)
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.True(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
Expand All @@ -173,7 +171,7 @@ func TestContainerUpdateCheck(t *testing.T) {
//new loki cert name
loki.TLS.CACert.Name = "cm-name-2"
spec = flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder = newBuilder(testNamespace, testImage, &spec, &certWatcher)
builder = newBuilder(testNamespace, testImage, &spec)
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.True(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
Expand All @@ -183,7 +181,7 @@ func TestContainerUpdateCheck(t *testing.T) {
//test again no change
loki.TLS.CACert.Name = "cm-name-2"
spec = flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder = newBuilder(testNamespace, testImage, &spec, &certWatcher)
builder = newBuilder(testNamespace, testImage, &spec)
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.False(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
Expand All @@ -195,7 +193,7 @@ func TestContainerUpdateCheck(t *testing.T) {
loki.StatusTLS.Enable = true

spec = flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder = newBuilder(testNamespace, testImage, &spec, &certWatcher)
builder = newBuilder(testNamespace, testImage, &spec)
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.True(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
Expand All @@ -210,7 +208,7 @@ func TestContainerUpdateCheck(t *testing.T) {
}

spec = flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder = newBuilder(testNamespace, testImage, &spec, &certWatcher)
builder = newBuilder(testNamespace, testImage, &spec)
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.True(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
Expand All @@ -226,7 +224,7 @@ func TestContainerUpdateCheck(t *testing.T) {
}

spec = flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder = newBuilder(testNamespace, testImage, &spec, &certWatcher)
builder = newBuilder(testNamespace, testImage, &spec)
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.True(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
Expand Down Expand Up @@ -264,7 +262,7 @@ func TestBuiltService(t *testing.T) {
plugin := getPluginConfig()
loki := flowslatest.FlowCollectorLoki{URL: "http://foo:1234"}
spec := flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder := newBuilder(testNamespace, testImage, &spec, &certWatcher)
builder := newBuilder(testNamespace, testImage, &spec)
newService := builder.service(nil)
report := helper.NewChangeReport("")
assert.Equal(serviceNeedsUpdate(newService, &plugin, &report), false)
Expand All @@ -277,7 +275,7 @@ func TestLabels(t *testing.T) {
plugin := getPluginConfig()
loki := flowslatest.FlowCollectorLoki{URL: "http://foo:1234"}
spec := flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder := newBuilder(testNamespace, testImage, &spec, &certWatcher)
builder := newBuilder(testNamespace, testImage, &spec)

// Deployment
depl := builder.deployment("digest")
Expand Down
7 changes: 1 addition & 6 deletions controllers/ebpf/agent_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,6 @@ func (c *AgentController) Reconcile(
}
desired := c.desired(target)

// Annotate pod with certificate reference so that it is reloaded if modified
if err := c.client.CertWatcher.AnnotatePod(ctx, c.client, &desired.Spec.Template, kafkaCerts); err != nil {
return err
}

switch c.requiredAction(current, desired) {
case actionCreate:
rlog.Info("action: create agent")
Expand Down Expand Up @@ -175,7 +170,7 @@ func (c *AgentController) desired(coll *flowslatest.FlowCollector) *v1.DaemonSet
if helper.UseKafka(&coll.Spec) && coll.Spec.Kafka.TLS.Enable {
// NOTE: secrets need to be copied from the base netobserv namespace to the privileged one.
// This operation must currently be performed manually (run "make fix-ebpf-kafka-tls"). It could be automated here.
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &coll.Spec.Kafka.TLS, kafkaCerts, c.client.CertWatcher)
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &coll.Spec.Kafka.TLS, kafkaCerts)
}

return &v1.DaemonSet{
Expand Down
3 changes: 1 addition & 2 deletions controllers/flowcollector_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,8 +368,7 @@ func (r *FlowCollectorReconciler) finalize(ctx context.Context, desired *flowsla

func (r *FlowCollectorReconciler) newClientHelper(desired *flowslatest.FlowCollector) reconcilers.ClientHelper {
return reconcilers.ClientHelper{
CertWatcher: r.certWatcher,
Client: r.Client,
Client: r.Client,
SetControllerReference: func(obj client.Object) error {
return ctrl.SetControllerReference(desired, obj, r.Scheme)
},
Expand Down
37 changes: 5 additions & 32 deletions controllers/flowcollector_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -593,9 +593,8 @@ func flowCollectorControllerSpecs() {
})
})

Context("Using and watching certificates", func() {
Context("Using certificates", func() {
flpDS := appsv1.DaemonSet{}
var certStamp1, certStamp2 string
It("Should update Loki to use TLS", func() {
// Create CM certificate
Expect(k8sClient.Create(ctx, &v1.ConfigMap{
Expand All @@ -622,33 +621,8 @@ func flowCollectorControllerSpecs() {
if err := k8sClient.Get(ctx, flpKey1, &flpDS); err != nil {
return err
}
certStamp1 = flpDS.Spec.Template.Annotations["flows.netobserv.io/cert-loki-certs-ca"]
return certStamp1
}, timeout, interval).Should(Not(BeEmpty()))
Expect(flpDS.Spec.Template.Spec.Volumes).To(HaveLen(2))
Expect(flpDS.Spec.Template.Spec.Volumes[0].Name).To(Equal("config-volume"))
Expect(flpDS.Spec.Template.Spec.Volumes[1].Name).To(Equal("loki-certs-ca"))
})

It("Should watch certificate update", func() {
By("Updating certificate")
Expect(k8sClient.Update(ctx, &v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "loki-ca",
Namespace: operatorNamespace,
},
Data: map[string]string{"test": "test"},
})).Should(Succeed())

Eventually(func() interface{} {
if err := k8sClient.Get(ctx, flpKey1, &flpDS); err != nil {
return err
}
certStamp2 = flpDS.Spec.Template.Annotations["flows.netobserv.io/cert-loki-certs-ca"]
return certStamp2
}, timeout, interval).Should(Not(Equal(certStamp1)))
Expect(certStamp2).To(Not(BeEmpty()))
Expect(flpDS.Spec.Template.Spec.Volumes).To(HaveLen(2))
return flpDS.Spec.Template.Spec.Volumes
}, timeout, interval).Should(HaveLen(2))
Expect(flpDS.Spec.Template.Spec.Volumes[0].Name).To(Equal("config-volume"))
Expect(flpDS.Spec.Template.Spec.Volumes[1].Name).To(Equal("loki-certs-ca"))
})
Expand All @@ -663,9 +637,8 @@ func flowCollectorControllerSpecs() {
if err := k8sClient.Get(ctx, flpKey1, &flpDS); err != nil {
return err
}
return flpDS.Spec.Template.Annotations
}, timeout, interval).Should(Not(HaveKey("flows.netobserv.io/cert-loki-certs-ca")))
Expect(flpDS.Spec.Template.Spec.Volumes).To(HaveLen(1))
return flpDS.Spec.Template.Spec.Volumes
}, timeout, interval).Should(HaveLen(1))
Expect(flpDS.Spec.Template.Spec.Volumes[0].Name).To(Equal("config-volume"))
})
})
Expand Down
11 changes: 4 additions & 7 deletions controllers/flowlogspipeline/flp_common_objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/netobserv/network-observability-operator/controllers/constants"
"github.com/netobserv/network-observability-operator/pkg/filters"
"github.com/netobserv/network-observability-operator/pkg/helper"
"github.com/netobserv/network-observability-operator/pkg/watchers"
)

const (
Expand Down Expand Up @@ -80,10 +79,9 @@ type builder struct {
confKind ConfKind
useOpenShiftSCC bool
image string
cWatcher *watchers.CertificatesWatcher
}

func newBuilder(ns, image string, desired *flowslatest.FlowCollectorSpec, ck ConfKind, useOpenShiftSCC bool, cWatcher *watchers.CertificatesWatcher) builder {
func newBuilder(ns, image string, desired *flowslatest.FlowCollectorSpec, ck ConfKind, useOpenShiftSCC bool) builder {
version := helper.ExtractVersion(image)
name := name(ck)
var promTLS flowslatest.CertificateReference
Expand Down Expand Up @@ -112,7 +110,6 @@ func newBuilder(ns, image string, desired *flowslatest.FlowCollectorSpec, ck Con
useOpenShiftSCC: useOpenShiftSCC,
promTLS: &promTLS,
image: image,
cWatcher: cWatcher,
}
}

Expand Down Expand Up @@ -185,20 +182,20 @@ func (b *builder) podTemplate(hasHostPort, hasLokiInterface, hostNetwork bool, c
}}

if helper.UseKafka(b.desired) && b.desired.Kafka.TLS.Enable {
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &b.desired.Kafka.TLS, kafkaCerts, b.cWatcher)
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &b.desired.Kafka.TLS, kafkaCerts)
}

if hasLokiInterface {
if b.desired.Loki.TLS.Enable && !b.desired.Loki.TLS.InsecureSkipVerify {
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &b.desired.Loki.TLS, lokiCerts, b.cWatcher)
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &b.desired.Loki.TLS, lokiCerts)
}
if helper.LokiUseHostToken(&b.desired.Loki) || helper.LokiForwardUserToken(&b.desired.Loki) {
volumes, volumeMounts = helper.AppendTokenVolume(volumes, volumeMounts, lokiToken, constants.FLPName)
}
}

if b.desired.Processor.Metrics.Server.TLS.Type != flowslatest.ServerTLSDisabled {
volumes, volumeMounts = helper.AppendSingleCertVolumes(volumes, volumeMounts, b.promTLS, promCerts, b.cWatcher)
volumes, volumeMounts = helper.AppendSingleCertVolumes(volumes, volumeMounts, b.promTLS, promCerts)
}

var envs []corev1.EnvVar
Expand Down
5 changes: 2 additions & 3 deletions controllers/flowlogspipeline/flp_ingest_objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@ import (
"github.com/netobserv/flowlogs-pipeline/pkg/config"
flowslatest "github.com/netobserv/network-observability-operator/api/v1beta1"
"github.com/netobserv/network-observability-operator/pkg/helper"
"github.com/netobserv/network-observability-operator/pkg/watchers"
)

type ingestBuilder struct {
generic builder
}

func newIngestBuilder(ns, image string, desired *flowslatest.FlowCollectorSpec, useOpenShiftSCC bool, cWatcher *watchers.CertificatesWatcher) ingestBuilder {
gen := newBuilder(ns, image, desired, ConfKafkaIngester, useOpenShiftSCC, cWatcher)
func newIngestBuilder(ns, image string, desired *flowslatest.FlowCollectorSpec, useOpenShiftSCC bool) ingestBuilder {
gen := newBuilder(ns, image, desired, ConfKafkaIngester, useOpenShiftSCC)
return ingestBuilder{
generic: gen,
}
Expand Down
6 changes: 1 addition & 5 deletions controllers/flowlogspipeline/flp_ingest_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (r *flpIngesterReconciler) reconcile(ctx context.Context, desired *flowslat
return nil
}

builder := newIngestBuilder(r.nobjMngr.Namespace, r.image, &desired.Spec, r.useOpenShiftSCC, r.CertWatcher)
builder := newIngestBuilder(r.nobjMngr.Namespace, r.image, &desired.Spec, r.useOpenShiftSCC)
newCM, configDigest, err := builder.configMap()
if err != nil {
return err
Expand Down Expand Up @@ -147,10 +147,6 @@ func (r *flpIngesterReconciler) reconcileDaemonSet(ctx context.Context, desiredD
report := helper.NewChangeReport("FLP DaemonSet")
defer report.LogIfNeeded(ctx)

// Annotate pod with certificate reference so that it is reloaded if modified
if err := r.CertWatcher.AnnotatePod(ctx, r.Client, &desiredDS.Spec.Template, lokiCerts, kafkaCerts); err != nil {
return err
}
if !r.nobjMngr.Exists(r.owned.daemonSet) {
return r.CreateOwned(ctx, desiredDS)
} else if helper.PodChanged(&r.owned.daemonSet.Spec.Template, &desiredDS.Spec.Template, constants.FLPName, &report) {
Expand Down
5 changes: 2 additions & 3 deletions controllers/flowlogspipeline/flp_monolith_objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@ import (
"github.com/netobserv/flowlogs-pipeline/pkg/config"
flowslatest "github.com/netobserv/network-observability-operator/api/v1beta1"
"github.com/netobserv/network-observability-operator/pkg/helper"
"github.com/netobserv/network-observability-operator/pkg/watchers"
)

type monolithBuilder struct {
generic builder
}

func newMonolithBuilder(ns, image string, desired *flowslatest.FlowCollectorSpec, useOpenShiftSCC bool, cWatcher *watchers.CertificatesWatcher) monolithBuilder {
gen := newBuilder(ns, image, desired, ConfMonolith, useOpenShiftSCC, cWatcher)
func newMonolithBuilder(ns, image string, desired *flowslatest.FlowCollectorSpec, useOpenShiftSCC bool) monolithBuilder {
gen := newBuilder(ns, image, desired, ConfMonolith, useOpenShiftSCC)
return monolithBuilder{
generic: gen,
}
Expand Down
6 changes: 1 addition & 5 deletions controllers/flowlogspipeline/flp_monolith_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (r *flpMonolithReconciler) reconcile(ctx context.Context, desired *flowslat
return nil
}

builder := newMonolithBuilder(r.nobjMngr.Namespace, r.image, &desired.Spec, r.useOpenShiftSCC, r.CertWatcher)
builder := newMonolithBuilder(r.nobjMngr.Namespace, r.image, &desired.Spec, r.useOpenShiftSCC)
newCM, configDigest, dbConfigMap, err := builder.configMap()
if err != nil {
return err
Expand Down Expand Up @@ -155,10 +155,6 @@ func (r *flpMonolithReconciler) reconcileDaemonSet(ctx context.Context, desiredD
report := helper.NewChangeReport("FLP DaemonSet")
defer report.LogIfNeeded(ctx)

// Annotate pod with certificate reference so that it is reloaded if modified
if err := r.CertWatcher.AnnotatePod(ctx, r.Client, &desiredDS.Spec.Template, lokiCerts, kafkaCerts); err != nil {
return err
}
if !r.nobjMngr.Exists(r.owned.daemonSet) {
return r.CreateOwned(ctx, desiredDS)
} else if helper.PodChanged(&r.owned.daemonSet.Spec.Template, &desiredDS.Spec.Template, constants.FLPName, &report) {
Expand Down