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.

Sign up for GitHub

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

feat: check dependency for state of the world reconciler #870

Merged
merged 1 commit into from
Sep 27, 2024
Merged

feat: check dependency for state of the world reconciler #870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 73 additions & 3 deletions controllers/state_of_the_world.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@ import (
"fmt"
"strings"

certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
egv1alpha1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp"
"github.com/kuadrant/policy-machinery/controller"
"github.com/kuadrant/policy-machinery/machinery"
istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1"
istioclientnetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
istioclientgosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand All @@ -20,6 +25,9 @@ import (
kuadrantv1alpha1 "github.com/kuadrant/kuadrant-operator/api/v1alpha1"
kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1"
kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2"
"github.com/kuadrant/kuadrant-operator/pkg/envoygateway"
"github.com/kuadrant/kuadrant-operator/pkg/istio"
kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi"
"github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant"
)

Expand All @@ -36,9 +44,6 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D
controller.WithLogger(logger),
controller.WithClient(client),
controller.WithRunnable("kuadrant watcher", controller.Watch(&kuadrantv1beta1.Kuadrant{}, kuadrantv1beta1.KuadrantResource, metav1.NamespaceAll)),
controller.WithRunnable("gatewayclass watcher", controller.Watch(&gwapiv1.GatewayClass{}, controller.GatewayClassesResource, metav1.NamespaceAll)),
controller.WithRunnable("gateway watcher", controller.Watch(&gwapiv1.Gateway{}, controller.GatewaysResource, metav1.NamespaceAll)),
controller.WithRunnable("httproute watcher", controller.Watch(&gwapiv1.HTTPRoute{}, controller.HTTPRoutesResource, metav1.NamespaceAll)),
controller.WithRunnable("dnspolicy watcher", controller.Watch(&kuadrantv1alpha1.DNSPolicy{}, kuadrantv1alpha1.DNSPoliciesResource, metav1.NamespaceAll)),
controller.WithRunnable("tlspolicy watcher", controller.Watch(&kuadrantv1alpha1.TLSPolicy{}, kuadrantv1alpha1.TLSPoliciesResource, metav1.NamespaceAll)),
controller.WithRunnable("authpolicy watcher", controller.Watch(&kuadrantv1beta2.AuthPolicy{}, kuadrantv1beta2.AuthPoliciesResource, metav1.NamespaceAll)),
Expand All @@ -59,6 +64,71 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D
controller.WithReconcile(buildReconciler(client)),
}

ok, err := kuadrantgatewayapi.IsGatewayAPIInstalled(manager.GetRESTMapper())
Copy link
Contributor

Choose a reason for hiding this comment

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

What should happen if there is no gateway currently installed that we can use? We would need at least one, I am asking the same question of the cert manger if that is not installed should we try reconcile. I am aware that the user may install these resources later, but that would mean restarting the operator (which I am okay with).

I guess my question is; should we even start the operator if there is no gateway that we can use installed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, not sure what's the best approach but I've went with the approach of the existing controllers which gets disabled rather than crash when a dependency CRD is not detected (which we have existing integration tests for) 🤔

Personally, I think it's better not to crash and have this reported in the Policy Status also since a user may not initially look through logs on why things are not working as expected.

Copy link
Contributor

@eguzki eguzki Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I agree, the operator should not crash. Personally, for now, what I would do is to disable watchers and maybe steps in the workflow. There should be a task responsible for the kuadrant CR status that watches for changes on Kuadrant CRDs (which we can assume they exist). Then, the task checks for GatewayAPI being installed and reports in the status if not. Same for policies.

Copy link
Contributor

Choose a reason for hiding this comment

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

After the call I am happy that we have a plan to get the errors these missing CRDs and resources can cause into the status of kuadrant and / or the related policies. Would be good to create issue for any follow up work this may need.

if err != nil || !ok {
logger.Info("gateway api is not installed, skipping watches and reconcilers", "err", err)
} else {
controllerOpts = append(controllerOpts,
controller.WithRunnable("gatewayclass watcher", controller.Watch(&gwapiv1.GatewayClass{}, controller.GatewayClassesResource, metav1.NamespaceAll)),
controller.WithRunnable("gateway watcher", controller.Watch(&gwapiv1.Gateway{}, controller.GatewaysResource, metav1.NamespaceAll)),
controller.WithRunnable("httproute watcher", controller.Watch(&gwapiv1.HTTPRoute{}, controller.HTTPRoutesResource, metav1.NamespaceAll)),
)
}

ok, err = envoygateway.IsEnvoyGatewayInstalled(manager.GetRESTMapper())
if err != nil || !ok {
logger.Info("envoygateway is not installed, skipping related watches and reconcilers", "err", err)
} else {
controllerOpts = append(controllerOpts,
controller.WithRunnable("envoypatchpolicy watcher", controller.Watch(&egv1alpha1.EnvoyPatchPolicy{}, envoygateway.EnvoyPatchPoliciesResource, metav1.NamespaceAll)),
controller.WithRunnable("envoyextensionpolicy watcher", controller.Watch(&egv1alpha1.EnvoyExtensionPolicy{}, envoygateway.EnvoyExtensionPoliciesResource, metav1.NamespaceAll)),
controller.WithRunnable("envoysecuritypolicy watcher", controller.Watch(&egv1alpha1.SecurityPolicy{}, envoygateway.SecurityPoliciesResource, metav1.NamespaceAll)),
controller.WithObjectKinds(
envoygateway.EnvoyPatchPolicyGroupKind,
envoygateway.EnvoyExtensionPolicyGroupKind,
envoygateway.SecurityPolicyGroupKind,
),
// TODO: add object links
)
// TODO: add specific tasks to workflow
Comment on lines +91 to +93
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Future TODOs that would be completed by other issues as functionality is migrated to the workflow

}

ok, err = istio.IsIstioInstalled(manager.GetRESTMapper())
if err != nil || !ok {
logger.Info("istio is not installed, skipping related watches and reconcilers", "err", err)
} else {
controllerOpts = append(controllerOpts,
controller.WithRunnable("envoyfilter watcher", controller.Watch(&istioclientnetworkingv1alpha3.EnvoyFilter{}, istio.EnvoyFiltersResource, metav1.NamespaceAll)),
controller.WithRunnable("wasmplugin watcher", controller.Watch(&istioclientgoextensionv1alpha1.WasmPlugin{}, istio.WasmPluginsResource, metav1.NamespaceAll)),
controller.WithRunnable("authorizationpolicy watcher", controller.Watch(&istioclientgosecurityv1beta1.AuthorizationPolicy{}, istio.AuthorizationPoliciesResource, metav1.NamespaceAll)),
controller.WithObjectKinds(
istio.EnvoyFilterGroupKind,
istio.WasmPluginGroupKind,
istio.AuthorizationPolicyGroupKind,
),
// TODO: add object links
)
// TODO: add istio specific tasks to workflow
}

ok, err = kuadrantgatewayapi.IsCertManagerInstalled(manager.GetRESTMapper(), logger)
if err != nil || !ok {
logger.Info("cert manager is not installed, skipping related watches and reconcilers", "err", err)
} else {
controllerOpts = append(controllerOpts,
controller.WithRunnable("certificate watcher", controller.Watch(&certmanagerv1.Certificate{}, CertManagerCertificatesResource, metav1.NamespaceAll)),
controller.WithRunnable("issuers watcher", controller.Watch(&certmanagerv1.Issuer{}, CertManagerIssuersResource, metav1.NamespaceAll)),
controller.WithRunnable("clusterissuers watcher", controller.Watch(&certmanagerv1.Certificate{}, CertMangerClusterIssuersResource, metav1.NamespaceAll)),
controller.WithObjectKinds(
CertManagerCertificateKind,
CertManagerIssuerKind,
CertManagerClusterIssuerKind,
),
// TODO: add object links
)
// TODO: add tls policy specific tasks to workflow
}

return controller.NewController(controllerOpts...)
}

Expand Down
8 changes: 7 additions & 1 deletion controllers/test_common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
istiosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
istioapis "istio.io/istio/operator/pkg/apis"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
istiov1alpha1 "maistra.io/istio-operator/api/v1alpha1"
Expand Down Expand Up @@ -256,9 +257,14 @@ func SetupKuadrantOperatorForTest(s *runtime.Scheme, cfg *rest.Config) {

Expect(err).NotTo(HaveOccurred())

dClient, err := dynamic.NewForConfig(mgr.GetConfig())
Expect(err).NotTo(HaveOccurred())

stateOfTheWorld := NewPolicyMachineryController(mgr, dClient, log.Log)

go func() {
defer GinkgoRecover()
err = mgr.Start(ctrl.SetupSignalHandler())
err = stateOfTheWorld.Start(ctrl.SetupSignalHandler())
Expect(err).ToNot(HaveOccurred())
}()
}
Expand Down
12 changes: 12 additions & 0 deletions controllers/tlspolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import (
"fmt"
"reflect"

"github.com/cert-manager/cert-manager/pkg/apis/certmanager"
certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime/schema"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -44,6 +46,16 @@ import (

const TLSPolicyFinalizer = "kuadrant.io/tls-policy"

var (
CertManagerCertificatesResource = certmanagerv1.SchemeGroupVersion.WithResource("certificates")
CertManagerIssuersResource = certmanagerv1.SchemeGroupVersion.WithResource("issuers")
CertMangerClusterIssuersResource = certmanagerv1.SchemeGroupVersion.WithResource("clusterissuers")

CertManagerCertificateKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.CertificateKind}
CertManagerIssuerKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.IssuerKind}
CertManagerClusterIssuerKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.ClusterIssuerKind}
)

// TLSPolicyReconciler reconciles a TLSPolicy object
type TLSPolicyReconciler struct {
*reconcilers.BaseReconciler
Expand Down
10 changes: 5 additions & 5 deletions make/integration-tests.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ INTEGRATION_TEST_NUM_PROCESSES ?= 10
test-bare-k8s-integration: clean-cov generate fmt vet ginkgo ## Requires only bare kubernetes cluster.
mkdir -p $(PROJECT_PATH)/coverage/bare-k8s-integration
# Check `ginkgo help run` for command line options. For example to filtering tests.
$(GINKGO) \
OPERATOR_NAMESPACE=kuadrant-system $(GINKGO) \
--coverpkg $(INTEGRATION_COVER_PKGS) \
--output-dir $(PROJECT_PATH)/coverage/bare-k8s-integration \
--coverprofile cover.out \
Expand All @@ -27,7 +27,7 @@ test-bare-k8s-integration: clean-cov generate fmt vet ginkgo ## Requires only ba
test-gatewayapi-env-integration: clean-cov generate fmt vet ginkgo ## Requires kubernetes cluster with GatewayAPI installed.
mkdir -p $(PROJECT_PATH)/coverage/gatewayapi-integration
# Check `ginkgo help run` for command line options. For example to filtering tests.
$(GINKGO) \
OPERATOR_NAMESPACE=kuadrant-system $(GINKGO) \
--coverpkg $(INTEGRATION_COVER_PKGS) \
--output-dir $(PROJECT_PATH)/coverage/gatewayapi-integration \
--coverprofile cover.out \
Expand All @@ -45,7 +45,7 @@ test-gatewayapi-env-integration: clean-cov generate fmt vet ginkgo ## Requires k
test-istio-env-integration: clean-cov generate fmt vet ginkgo ## Requires kubernetes cluster with GatewayAPI and Istio installed.
mkdir -p $(PROJECT_PATH)/coverage/istio-integration
# Check `ginkgo help run` for command line options. For example to filtering tests.
GATEWAYAPI_PROVIDER=istio $(GINKGO) \
GATEWAYAPI_PROVIDER=istio OPERATOR_NAMESPACE=kuadrant-system $(GINKGO) \
--coverpkg $(INTEGRATION_COVER_PKGS) \
--output-dir $(PROJECT_PATH)/coverage/istio-integration \
--coverprofile cover.out \
Expand All @@ -62,7 +62,7 @@ test-istio-env-integration: clean-cov generate fmt vet ginkgo ## Requires kubern
test-envoygateway-env-integration: clean-cov generate fmt vet ginkgo ## Requires kubernetes cluster with GatewayAPI and EnvoyGateway installed.
mkdir -p $(PROJECT_PATH)/coverage/envoygateway-integration
# Check `ginkgo help run` for command line options. For example to filtering tests.
GATEWAYAPI_PROVIDER=envoygateway $(GINKGO) \
GATEWAYAPI_PROVIDER=envoygateway OPERATOR_NAMESPACE=kuadrant-system $(GINKGO) \
--coverpkg $(INTEGRATION_COVER_PKGS) \
--output-dir $(PROJECT_PATH)/coverage/envoygateway-integration \
--coverprofile cover.out \
Expand All @@ -80,7 +80,7 @@ test-envoygateway-env-integration: clean-cov generate fmt vet ginkgo ## Requires
test-integration: clean-cov generate fmt vet ginkgo ## Requires kubernetes cluster with at least one GatewayAPI provider installed.
mkdir -p $(PROJECT_PATH)/coverage/integration
# Check `ginkgo help run` for command line options. For example to filtering tests.
GATEWAYAPI_PROVIDER=$(GATEWAYAPI_PROVIDER) $(GINKGO) \
GATEWAYAPI_PROVIDER=$(GATEWAYAPI_PROVIDER) OPERATOR_NAMESPACE=kuadrant-system $(GINKGO) \
--coverpkg $(INTEGRATION_COVER_PKGS) \
--output-dir $(PROJECT_PATH)/coverage/integration \
--coverprofile cover.out \
Expand Down
11 changes: 11 additions & 0 deletions pkg/envoygateway/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,21 @@ package envoygateway
import (
egv1alpha1 "github.com/envoyproxy/gateway/api/v1alpha1"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime/schema"

kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi"
)

var (
EnvoyPatchPoliciesResource = egv1alpha1.SchemeBuilder.GroupVersion.WithResource("envoypatchpolicies")
EnvoyExtensionPoliciesResource = egv1alpha1.SchemeBuilder.GroupVersion.WithResource("envoyextensionpolicies")
SecurityPoliciesResource = egv1alpha1.SchemeBuilder.GroupVersion.WithResource("securitypolicies")

EnvoyPatchPolicyGroupKind = schema.GroupKind{Group: egv1alpha1.GroupName, Kind: egv1alpha1.KindEnvoyPatchPolicy}
EnvoyExtensionPolicyGroupKind = schema.GroupKind{Group: egv1alpha1.GroupName, Kind: egv1alpha1.KindEnvoyExtensionPolicy}
SecurityPolicyGroupKind = schema.GroupKind{Group: egv1alpha1.GroupName, Kind: egv1alpha1.KindSecurityPolicy}
)

func IsEnvoyPatchPolicyInstalled(restMapper meta.RESTMapper) (bool, error) {
return kuadrantgatewayapi.IsCRDInstalled(
restMapper,
Expand Down
11 changes: 11 additions & 0 deletions pkg/istio/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,23 @@ import (
istioclientnetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
istioclientgosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"

kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi"
)

var (
EnvoyFiltersResource = istioclientnetworkingv1alpha3.SchemeGroupVersion.WithResource("envoyfilters")
WasmPluginsResource = istioclientgoextensionv1alpha1.SchemeGroupVersion.WithResource("wasmplugins")
AuthorizationPoliciesResource = istioclientgosecurityv1beta1.SchemeGroupVersion.WithResource("authorizationpolicies")

EnvoyFilterGroupKind = schema.GroupKind{Group: istioclientnetworkingv1alpha3.GroupName, Kind: "EnvoyFilter"}
WasmPluginGroupKind = schema.GroupKind{Group: istioclientgoextensionv1alpha1.GroupName, Kind: "WasmPlugin"}
AuthorizationPolicyGroupKind = schema.GroupKind{Group: istioclientgosecurityv1beta1.GroupName, Kind: "AuthorizationPolicy"}
)

func WorkloadSelectorFromGateway(ctx context.Context, k8sClient client.Client, gateway *gatewayapiv1.Gateway) *istiocommon.WorkloadSelector {
logger, _ := logr.FromContext(ctx)
gatewayWorkloadSelector, err := kuadrantgatewayapi.GetGatewayWorkloadSelector(ctx, k8sClient, gateway)
Expand Down
Loading