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

pre 0.8.0 flagd-proxy service does not contain managed-by label #719

Open
xvzf opened this issue Oct 30, 2024 · 3 comments
Open

pre 0.8.0 flagd-proxy service does not contain managed-by label #719

xvzf opened this issue Oct 30, 2024 · 3 comments

Comments

@xvzf
Copy link
Contributor

xvzf commented Oct 30, 2024

Changes introduced in #712 do not only validate the ownership of the flagd-proxy deployment but also the service, which leads to the service not being managed further by the OFO.

This only affects installations prior to 0.8.0 with flagd-proxy enabled.

A fix for this issue is to manually label the affected resource:

kubectl label -n open-feature-operator-system svc/flagd-proxy-svc app.kubernetes.io/managed-by=open-feature-operator
@xvzf
Copy link
Contributor Author

xvzf commented Oct 30, 2024

@beeme1mr I personally think it's sufficient to add a warning to the 0.8.0 release for this as a breaking change, wdyt?

@wondertroy
Copy link
Contributor

wondertroy commented Oct 31, 2024

Does ownership have to be validated via checking for that label?

I can see in a previous version of the Service resource for flagd-proxy that it contains ownerReferences in the metadata, and the managed-by field within the selector in the spec. I'm making naive assumptions that these fields are all injected and accessible so might not be possible, but wondering if they could be used to validate ownership instead of the label?

Eg: Older version Service resource pre- 0.8.0

apiVersion: v1
kind: Service
metadata:
  creationTimestamp: XXXX-XX-XXTXX:XX:17Z
  name: flagd-proxy-svc
  namespace: XXXX
  ownerReferences:
    - apiVersion: apps/v1
      kind: Deployment
      name: open-feature-operator-controller-manager # <-- here
      uid: XXXXXXXXXX
  resourceVersion: "XXXX"
  uid: 689f2c91-b183-4d4f-a295-26f27402b5d6
spec:
  clusterIP: XX.XXX.XXX.XX
  clusterIPs:
    - XX.XXX.XXX.XX
  internalTrafficPolicy: Cluster
  ipFamilies:
    - IPv4
  ipFamilyPolicy: SingleStack
  ports:
    - xxx
  selector:
    app.kubernetes.io/managed-by: open-feature-operator # <-- here
    app.kubernetes.io/name: flagd-proxy
  sessionAffinity: None
  type: ClusterIP
status:
  loadBalancer: {}

New service resource post 0.8.0 update:

apiVersion: v1
kind: Service
metadata:
  creationTimestamp:XXXX-XX-XXTXX:XX:17Z
  labels:
    app.kubernetes.io/managed-by: open-feature-operator # <-- new / easily added manually if needed
  name: flagd-proxy-svc
  namespace: flags-system
  ownerReferences:
    - apiVersion: apps/v1
      kind: Deployment
      name: open-feature-operator-controller-manager
      uid: xxxxxx
  resourceVersion: "xxxxxx"
  uid: xxxxx xx
spec:
  clusterIP: xx.xxx.xxx.xxx
  clusterIPs:
    - xx.xxx.xxx.xxx
  internalTrafficPolicy: Cluster
  ipFamilies:
    - IPv4
  ipFamilyPolicy: SingleStack
  ports:
    - xxxx
  selector:
    app.kubernetes.io/managed-by: open-feature-operator
    app.kubernetes.io/name: flagd-proxy
  sessionAffinity: None
  type: ClusterIP
status:
  loadBalancer: {}

Example diff:

Index: common/common.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/common/common.go b/common/common.go
--- a/common/common.go	(revision bcdafd29a0bd5a64f8de35bfe573566155021226)
+++ b/common/common.go	(date 1730333262536)
@@ -84,5 +84,8 @@
 
 func IsManagedByOFO(obj client.Object) bool {
 	val, ok := obj.GetLabels()[ManagedByAnnotationKey]
-	return ok && val == ManagedByAnnotationValue
+	ownerRefs := obj.GetOwnerReferences()
+	return ok && val == ManagedByAnnotationValue ||
+		len(ownerRefs) == 1 &&
+			ownerRefs[0].Name == OperatorDeploymentName
 }

@xvzf
Copy link
Contributor Author

xvzf commented Oct 31, 2024

I'd personally use the ownerReference as well, but I assumed the decision to use a label has been taken on purpose.

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

No branches or pull requests

2 participants