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

Use openshift base domain if ingress hostname is missing #2059

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 16 additions & 0 deletions .chloggen/ocp-domain.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
component: operator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Default to OpenShift base domain if ingress hostname is missing

# One or more tracking issues related to the change
issues: [2059]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,14 @@ spec:
- patch
- update
- watch
- apiGroups:
- config.openshift.io
resources:
- dnses
verbs:
- get
- list
- watch
- apiGroups:
- coordination.k8s.io
resources:
Expand Down Expand Up @@ -281,6 +289,14 @@ spec:
- get
- patch
- update
- apiGroups:
- operator.openshift.io
resources:
- ingresscontrollers
verbs:
- get
- list
- watch
- apiGroups:
- route.openshift.io
resources:
Expand Down
16 changes: 16 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,14 @@ rules:
- patch
- update
- watch
- apiGroups:
- config.openshift.io
resources:
- dnses
verbs:
- get
- list
- watch
- apiGroups:
- coordination.k8s.io
resources:
Expand Down Expand Up @@ -179,6 +187,14 @@ rules:
- get
- patch
- update
- apiGroups:
- operator.openshift.io
resources:
- ingresscontrollers
verbs:
- get
- list
- watch
- apiGroups:
- route.openshift.io
resources:
Expand Down
2 changes: 2 additions & 0 deletions controllers/opentelemetrycollector_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ func NewReconciler(p Params) *OpenTelemetryCollectorReconciler {
// +kubebuilder:rbac:groups=opentelemetry.io,resources=opentelemetrycollectors/finalizers,verbs=get;update;patch
// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=route.openshift.io,resources=routes;routes/custom-host,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=operator.openshift.io,resources=ingresscontrollers,verbs=get;list;watch
// +kubebuilder:rbac:groups=config.openshift.io,resources=dnses,verbs=get;list;watch
// +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;create;update
// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch

Expand Down
10 changes: 6 additions & 4 deletions controllers/opentelemetrycollector_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
"k8s.io/kubectl/pkg/scheme"
"sigs.k8s.io/controller-runtime/pkg/client"
k8sconfig "sigs.k8s.io/controller-runtime/pkg/client/config"
Expand Down Expand Up @@ -60,10 +61,11 @@ func TestNewObjectsOnReconciliation(t *testing.T) {
)
nsn := types.NamespacedName{Name: "my-instance", Namespace: "default"}
reconciler := controllers.NewReconciler(controllers.Params{
Client: k8sClient,
Log: logger,
Scheme: testScheme,
Config: cfg,
Client: k8sClient,
Log: logger,
Scheme: testScheme,
Config: cfg,
Recorder: record.NewFakeRecorder(10),
frzifus marked this conversation as resolved.
Show resolved Hide resolved
})
require.NoError(t, cfg.AutoDetect())
created := &v1alpha1.OpenTelemetryCollector{
Expand Down
16 changes: 14 additions & 2 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"testing"
"time"

configv1 "github.com/openshift/api/config/v1"
openshiftoperatorv1 "github.com/openshift/api/operator/v1"
routev1 "github.com/openshift/api/route/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -59,7 +61,7 @@ func TestMain(m *testing.M) {

testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")},
CRDs: []*apiextensionsv1.CustomResourceDefinition{testdata.OpenShiftRouteCRD},
CRDs: []*apiextensionsv1.CustomResourceDefinition{testdata.OpenShiftRouteCRD, testdata.OpenShiftIngressControllerCRD, testdata.OpenShiftConfigDNSCRD},
WebhookInstallOptions: envtest.WebhookInstallOptions{
Paths: []string{filepath.Join("..", "config", "webhook")},
},
Expand All @@ -70,7 +72,17 @@ func TestMain(m *testing.M) {
os.Exit(1)
}

if err = routev1.AddToScheme(testScheme); err != nil {
if err = routev1.Install(testScheme); err != nil {
fmt.Printf("failed to register scheme: %v", err)
os.Exit(1)
}

if err = openshiftoperatorv1.Install(testScheme); err != nil {
fmt.Printf("failed to register scheme: %v", err)
os.Exit(1)
}

if err = configv1.Install(testScheme); err != nil {
fmt.Printf("failed to register scheme: %v", err)
os.Exit(1)
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/Masterminds/semver/v3 v3.2.1
github.com/go-logr/logr v1.2.4
github.com/mitchellh/mapstructure v1.5.0
github.com/openshift/api v3.9.0+incompatible
github.com/openshift/api v0.0.0-20230825100711-f9d67cd677c9
github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.67.1
github.com/prometheus/prometheus v0.46.0
github.com/spf13/pflag v1.0.5
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,8 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
github.com/opencontainers/image-spec v1.0.2 h1:9yCKha/T5XdGtO0q9Q9a6T5NUCsTn/DrBg0D7ufOcFM=
github.com/opencontainers/image-spec v1.0.2/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
github.com/openshift/api v0.0.0-20230825100711-f9d67cd677c9 h1:biCIGGcc6x2K2Skdyu+lz/ZHBwOM592cdddoOZy23R8=
github.com/openshift/api v0.0.0-20230825100711-f9d67cd677c9/go.mod h1:yimSGmjsI+XF1mr+AKBs2//fSXIOhhetHGbMlBEfXbs=
github.com/openshift/api v3.9.0+incompatible h1:fJ/KsefYuZAjmrr3+5U9yZIZbTOpVkDDLDLFresAeYs=
github.com/openshift/api v3.9.0+incompatible/go.mod h1:dh9o4Fs58gpFXGSYfnVxGR9PnV53I8TW84pQaJDdGiY=
github.com/ovh/go-ovh v1.4.1 h1:VBGa5wMyQtTP7Zb+w97zRCh9sLtM/2YKRyy+MEJmWaM=
Expand Down
47 changes: 47 additions & 0 deletions internal/handlers/openshift/basedomain.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package openshift

import (
"context"
"fmt"

configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// GetOpenShiftBaseDomain returns base domain of OCP cluster.
func GetOpenShiftBaseDomain(ctx context.Context, k8sClient client.Client) (string, error) {
ictrl := &operatorv1.IngressController{}
err := k8sClient.Get(ctx, types.NamespacedName{Name: "default", Namespace: "openshift-ingress-operator"}, ictrl)
if err != nil {
// The preferred way to get the base domain is via OCP ingress controller
// this approach works well with CRC and normal OCP cluster.
// Fallback on cluster DNS might not work with CRC because CRC uses .apps-crc. and not .apps.
var clusterDNS configv1.DNS
if errDNS := k8sClient.Get(ctx, client.ObjectKey{Name: "cluster"}, &clusterDNS); errDNS != nil {
if apierrors.IsNotFound(errDNS) {
return "", fmt.Errorf("missing OpenShift IngressController and cluster DNS configuration to read base domain: %w", err)
}
return "", fmt.Errorf("failed to lookup base domain: %w", err)
}

return clusterDNS.Spec.BaseDomain, nil
}
return ictrl.Status.Domain, nil
}
50 changes: 50 additions & 0 deletions internal/handlers/openshift/basedomain_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package openshift

import (
"context"
"testing"

configv1 "github.com/openshift/api/config/v1"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestDomain(t *testing.T) {
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "openshift-ingress-operator",
},
}
err := k8sClient.Create(context.Background(), ns)
require.NoError(t, err)

clusterDNS := &configv1.DNS{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: configv1.DNSSpec{
BaseDomain: "test.crc",
},
}
err = k8sClient.Create(context.Background(), clusterDNS)
require.NoError(t, err)
domain, err := GetOpenShiftBaseDomain(context.Background(), k8sClient)
require.NoError(t, err)
assert.Equal(t, "test.crc", domain)
}
Loading