Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 48 additions & 17 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@ import (
"net/http"
"os"
"path/filepath"
"strings"
"time"

"github.com/spf13/pflag"
"go.uber.org/zap/zapcore"
corev1 "k8s.io/api/core/v1"
apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
k8slabels "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/selection"
k8stypes "k8s.io/apimachinery/pkg/types"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
_ "k8s.io/client-go/plugin/pkg/client/auth"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -52,7 +56,6 @@ import (
"github.com/operator-framework/operator-controller/internal/contentmanager"
"github.com/operator-framework/operator-controller/internal/controllers"
"github.com/operator-framework/operator-controller/internal/httputil"
"github.com/operator-framework/operator-controller/internal/labels"
"github.com/operator-framework/operator-controller/internal/resolve"
"github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety"
"github.com/operator-framework/operator-controller/internal/rukpak/source"
Expand Down Expand Up @@ -87,6 +90,7 @@ func main() {
operatorControllerVersion bool
systemNamespace string
caCertDir string
globalPullSecret string
)
flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
Expand All @@ -97,6 +101,7 @@ func main() {
flag.StringVar(&cachePath, "cache-path", "/var/cache", "The local directory path used for filesystem based caching")
flag.BoolVar(&operatorControllerVersion, "version", false, "Prints operator-controller version information")
flag.StringVar(&systemNamespace, "system-namespace", "", "Configures the namespace that gets used to deploy system resources.")
flag.StringVar(&globalPullSecret, "global-pull-secret", "", "The <namespace>/<name> of the global pull secret that is going to be used to pull bundle images.")
opts := zap.Options{
Development: true,
TimeEncoder: zapcore.RFC3339NanoTimeEncoder,
Expand All @@ -115,16 +120,42 @@ func main() {
ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts), zap.StacktraceLevel(zapcore.DPanicLevel)))
setupLog.Info("starting up the controller", "version info", version.String())

var globalPullSecretKey *k8stypes.NamespacedName
if globalPullSecret != "" {
secretParts := strings.Split(globalPullSecret, "/")
if len(secretParts) != 2 {
setupLog.Error(fmt.Errorf("incorrect number of components"), "value of global-pull-secret should be of the format <namespace>/<name>")
os.Exit(1)
}
globalPullSecretKey = &k8stypes.NamespacedName{Name: secretParts[1], Namespace: secretParts[0]}
}

if systemNamespace == "" {
systemNamespace = podNamespace()
}

dependentRequirement, err := k8slabels.NewRequirement(labels.OwnerKindKey, selection.In, []string{ocv1alpha1.ClusterExtensionKind})
if err != nil {
setupLog.Error(err, "unable to create dependent label selector for cache")
os.Exit(1)
cacheOptions := crcache.Options{
ByObject: map[client.Object]crcache.ByObject{
&ocv1alpha1.ClusterExtension{}: {Label: k8slabels.Everything()},
&catalogd.ClusterCatalog{}: {Label: k8slabels.Everything()},
},
DefaultNamespaces: map[string]crcache.Config{
systemNamespace: {LabelSelector: k8slabels.Everything()},
},
Copy link
Member

Choose a reason for hiding this comment

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

main also has DefaultLabelSelector: k8slabels.Nothing(), in the cache options. IIRC, that was an important detail.

Copy link
Member

Choose a reason for hiding this comment

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

@ankitathomas looks like this needs to fixed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea I'm confused by this configuration here too. @joelanford @ankitathomas could you elaborate on this configuration? Will help in resolving this issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've already pushed the fix on line 145

Besides the DefaultLabelSelector @joelanford pointed out, this is exactly the same as the cacheOptions in use prior to this PR

DefaultLabelSelector: k8slabels.Nothing(),
}
if globalPullSecretKey != nil {
cacheOptions.ByObject[&corev1.Secret{}] = crcache.ByObject{
Namespaces: map[string]crcache.Config{
globalPullSecretKey.Namespace: {
LabelSelector: k8slabels.Everything(),
FieldSelector: fields.SelectorFromSet(map[string]string{
"metadata.name": globalPullSecretKey.Name,
}),
},
},
}
}
dependentSelector := k8slabels.NewSelector().Add(*dependentRequirement)

setupLog.Info("set up manager")
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
Expand All @@ -133,16 +164,7 @@ func main() {
HealthProbeBindAddress: probeAddr,
LeaderElection: enableLeaderElection,
LeaderElectionID: "9c4404e7.operatorframework.io",
Cache: crcache.Options{
ByObject: map[client.Object]crcache.ByObject{
&ocv1alpha1.ClusterExtension{}: {Label: k8slabels.Everything()},
&catalogd.ClusterCatalog{}: {Label: k8slabels.Everything()},
},
DefaultNamespaces: map[string]crcache.Config{
systemNamespace: {LabelSelector: k8slabels.Everything()},
},
DefaultLabelSelector: dependentSelector,
},
Cache: cacheOptions,
// LeaderElectionReleaseOnCancel defines if the leader should step down voluntarily
// when the Manager ends. This requires the binary to immediately end when the
// Manager is stopped, otherwise, this setting is unsafe. Setting this significantly
Expand Down Expand Up @@ -200,6 +222,15 @@ func main() {
AuthNamespace: systemNamespace,
CertPoolWatcher: certPoolWatcher,
}
if globalPullSecretKey != nil {
unpacker.PullSecretFetcher = func(ctx context.Context) ([]corev1.Secret, error) {
pullSecret, err := coreClient.Secrets(globalPullSecretKey.Namespace).Get(ctx, globalPullSecretKey.Name, metav1.GetOptions{})
if err != nil {
return nil, err
}
return []corev1.Secret{*pullSecret}, err
}
}

clusterExtensionFinalizers := crfinalizer.NewFinalizers()
domain := ocv1alpha1.GroupVersion.Group
Expand Down
24 changes: 21 additions & 3 deletions internal/rukpak/source/image_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
gcrkube "github.com/google/go-containerregistry/pkg/authn/kubernetes"
"github.com/google/go-containerregistry/pkg/name"
"github.com/google/go-containerregistry/pkg/v1/remote"
corev1 "k8s.io/api/core/v1"
apimacherrors "k8s.io/apimachinery/pkg/util/errors"
"sigs.k8s.io/controller-runtime/pkg/log"

Expand Down Expand Up @@ -52,11 +53,14 @@ func NewUnrecoverable(err error) *Unrecoverable {
// TODO: Make asynchronous

type ImageRegistry struct {
BaseCachePath string
AuthNamespace string
CertPoolWatcher *httputil.CertPoolWatcher
BaseCachePath string
AuthNamespace string
CertPoolWatcher *httputil.CertPoolWatcher
PullSecretFetcher PullSecretFetcher
}

type PullSecretFetcher func(ctx context.Context) ([]corev1.Secret, error)

func (i *ImageRegistry) Unpack(ctx context.Context, bundle *BundleSource) (*Result, error) {
l := log.FromContext(ctx)
if bundle.Type != SourceTypeImage {
Expand Down Expand Up @@ -119,6 +123,20 @@ func (i *ImageRegistry) Unpack(ctx context.Context, bundle *BundleSource) (*Resu
}
}

if i.PullSecretFetcher != nil {
pullSecrets, err := i.PullSecretFetcher(ctx)
if err != nil {
l.V(1).Error(err, "failed to fetch global pullsecret, attempting unauthenticated image pull")
} else {
pullSecretAuth, err := gcrkube.NewFromPullSecrets(ctx, pullSecrets)
if err != nil {
l.V(1).Error(err, "failed to parse global pullsecret, attempting unauthenticated image pull")
} else {
remoteOpts = append(remoteOpts, remote.WithAuthFromKeychain(pullSecretAuth))
}
}
}

// always fetch the hash
imgDesc, err := remote.Head(imgRef, remoteOpts...)
if err != nil {
Expand Down
35 changes: 30 additions & 5 deletions openshift/generate-manifests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@ IMAGE_MAPPINGS[kube-rbac-proxy]='${KUBE_RBAC_PROXY_IMAGE}'
# shellcheck disable=SC2016
IMAGE_MAPPINGS[manager]='${OPERATOR_CONTROLLER_IMAGE}'

# This is a mapping of catalogd flag names to values. For example, given a deployment with a container
# named "manager" and arguments:
# args:
# - --flagname=one
# and an entry to the FLAG_MAPPINGS of FLAG_MAPPINGS[flagname]='two', the argument will be updated to:
# args:
# - --flagname=two
#
# If the flag doesn't already exist - it will be appended to the list.
declare -A FLAG_MAPPINGS
# shellcheck disable=SC2016
FLAG_MAPPINGS[global-pull-secret]="openshift-config/pull-secret"

##################################################
# You shouldn't need to change anything below here
##################################################
Expand All @@ -36,19 +49,20 @@ TMP_ROOT="$(mktemp -p . -d 2>/dev/null || mktemp -d ./tmpdir.XXXXXXX)"
trap 'rm -rf $TMP_ROOT' EXIT

# Copy all kustomize files into a temp dir
TMP_CONFIG="${TMP_ROOT}/config"
cp -a "${REPO_ROOT}/config" "$TMP_CONFIG"
cp -a "${REPO_ROOT}/config" "${TMP_ROOT}/config"
mkdir -p "${TMP_ROOT}/openshift"
cp -a "${REPO_ROOT}/openshift/kustomize" "${TMP_ROOT}/openshift/kustomize"

# Override namespace to openshift-operator-controller
$YQ -i ".namespace = \"${NAMESPACE}\"" "${TMP_CONFIG}/base/kustomization.yaml"
# Override OPENSHIFT-NAMESPACE to ${NAMESPACE}
find "${TMP_ROOT}" -name "*.yaml" -exec sed -i "s/OPENSHIFT-NAMESPACE/${NAMESPACE}/g" {} \;

# Create a temp dir for manifests
TMP_MANIFEST_DIR="${TMP_ROOT}/manifests"
mkdir -p "$TMP_MANIFEST_DIR"

# Run kustomize, which emits a single yaml file
TMP_KUSTOMIZE_OUTPUT="${TMP_MANIFEST_DIR}/temp.yaml"
$KUSTOMIZE build "${REPO_ROOT}"/openshift/kustomize/overlays/openshift -o "$TMP_KUSTOMIZE_OUTPUT"
$KUSTOMIZE build "${TMP_ROOT}/openshift/kustomize/overlays/openshift" -o "$TMP_KUSTOMIZE_OUTPUT"

for container_name in "${!IMAGE_MAPPINGS[@]}"; do
placeholder="${IMAGE_MAPPINGS[$container_name]}"
Expand All @@ -59,6 +73,17 @@ for container_name in "${!IMAGE_MAPPINGS[@]}"; do
$YQ -i 'select(.kind == "Namespace").metadata.annotations += {"workload.openshift.io/allowed": "management"}' "$TMP_KUSTOMIZE_OUTPUT"
done

# Loop through any flag updates that need to be made to the manager container
for flag_name in "${!FLAG_MAPPINGS[@]}"; do
flagval="${FLAG_MAPPINGS[$flag_name]}"

# First, update the flag if it exists
$YQ -i "(select(.kind == \"Deployment\") | .spec.template.spec.containers[] | select(.name == \"manager\") | .args[] | select(. | contains(\"--$flag_name=\")) | .) = \"--$flag_name=$flagval\"" "$TMP_KUSTOMIZE_OUTPUT"

# Then, append the flag if it doesn't exist
$YQ -i "(select(.kind == \"Deployment\") | .spec.template.spec.containers[] | select(.name == \"manager\") | .args) |= (select(.[] | contains(\"--$flag_name=\")) | .) // . + [\"--$flag_name=$flagval\"]" "$TMP_KUSTOMIZE_OUTPUT"
done

# Use yq to split the single yaml file into 1 per document.
# Naming convention: $index-$kind-$namespace-$name. If $namespace is empty, just use the empty string.
(
Expand Down
15 changes: 2 additions & 13 deletions openshift/kustomize/overlays/openshift/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,16 +1,5 @@
# Adds namespace to all resources.
namespace: openshift-operator-controller

namePrefix: operator-controller-

resources:
- resources/ca_configmap.yaml
- ../../../../config/base/crd
- ../../../../config/base/rbac
- ../../../../config/base/manager

patches:
- target:
kind: Deployment
name: controller-manager
path: patches/manager_deployment_ca.yaml
- olmv1-ns
- openshift-config
14 changes: 14 additions & 0 deletions openshift/kustomize/overlays/openshift/olmv1-ns/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Adds namespace to all resources.
namespace: OPENSHIFT-NAMESPACE

resources:
- resources/ca_configmap.yaml
- ../../../../../config/base/crd
- ../../../../../config/base/rbac
- ../../../../../config/base/manager

patches:
- target:
kind: Deployment
name: controller-manager
path: patches/manager_deployment_ca.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Adds namespace to all resources.
namespace: openshift-config

resources:
- rbac/operator-controller_manager_role.yaml
- rbac/operator-controller_manager_role_binding.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# permissions to do leader election.
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
labels:
app.kubernetes.io/part-of: olm
app.kubernetes.io/name: catalogd
name: manager-role
rules:
- apiGroups:
- ""
resources:
- secrets
verbs:
- get
- list
- watch
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
labels:
app.kubernetes.io/part-of: olm
app.kubernetes.io/name: catalogd
name: manager-rolebinding
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: manager-role
subjects:
- kind: ServiceAccount
name: controller-manager
namespace: OPENSHIFT-NAMESPACE
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
labels:
app.kubernetes.io/name: catalogd
app.kubernetes.io/part-of: olm
name: operator-controller-manager-role
namespace: openshift-config
rules:
- apiGroups:
- ""
resources:
- secrets
verbs:
- get
- list
- watch
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
labels:
app.kubernetes.io/name: catalogd
app.kubernetes.io/part-of: olm
name: operator-controller-manager-rolebinding
namespace: openshift-config
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: operator-controller-manager-role
subjects:
- kind: ServiceAccount
name: operator-controller-controller-manager
namespace: openshift-operator-controller
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ spec:
- --metrics-bind-address=127.0.0.1:8080
- --leader-elect
- --ca-certs-dir=/var/certs
- --global-pull-secret=openshift-config/pull-secret
command:
- /manager
image: ${OPERATOR_CONTROLLER_IMAGE}
Expand Down