[seaweedfs] Add External topology - #1231

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

packages/extra/seaweedfs/README.md (1)

16-16: Remove leaked “object” type hint to unblock CI

make generate failed because the line still contains the stray word “object”. Drop it so the description reads naturally:

-| `externalClusters`  | object A map of external clusters for managing bucketclaims.                                           | `{}`     |
+| `externalClusters`  | A map of external clusters for managing bucketclaims.                                                 | `{}`     |

packages/extra/seaweedfs/templates/dashboard-resourcemap.yaml (1)

52-63: Duplicate RoleBinding names will break kubectl apply

Both RoleBindings are called {{ .Release.Name }}-dashboard-resources.
Kubernetes forbids two objects with the same kind/name in one namespace; the second apply will fail.

-metadata:
-  name: {{ .Release.Name }}-dashboard-resources
+metadata:
+  name: {{ .Release.Name }}-dashboard-resources-superadmin

(Adjust roleRef/subjects accordingly.)

Also reconsider the privilege escalation to super-admin flagged in earlier review unless strictly required.

packages/extra/seaweedfs/templates/external/cosi-bucket-class.yaml (1)

1-16: YAML invalid & driverName mismatch

1. driverName/deletionPolicy (BucketClass) and driverName/authenticationType (BucketAccessClass) must live under spec: per COSI API.
2. BucketAccessClass.driverName should match BucketClass (<ns>.seaweedfs.objectstorage.k8s.io), not just the namespace.
3. Static analysis flags a top-level --- after a template directive – indentation must start at column 0.

Minimal fix:

---
kind: BucketClass
apiVersion: objectstorage.k8s.io/v1alpha1
metadata:
  name: {{ .Release.Namespace }}
-spec:
-  driverName: {{ .Release.Namespace }}.seaweedfs.objectstorage.k8s.io
-  deletionPolicy: Delete
+spec:
+  driverName: {{ .Release.Namespace }}.seaweedfs.objectstorage.k8s.io
+  deletionPolicy: Delete
---
kind: BucketAccessClass
apiVersion: objectstorage.k8s.io/v1alpha1
metadata:
  name: {{ .Release.Namespace }}
-spec:
-  driverName: {{ .Release.Namespace }}
-  authenticationType: KEY
+spec:
+  driverName: {{ .Release.Namespace }}.seaweedfs.objectstorage.k8s.io
+  authenticationType: KEY

Without these changes Flux will render invalid YAML and COSI will refuse the objects.

packages/extra/seaweedfs/Makefile (1)

5-8: Enum reinjection looks good but consider co-locating all post-processing here

The yq line correctly puts the "External" value back into values.schema.json (👍 for following the readme-generator quirk we’ve seen before).

If we ever need to patch other schema bits (e.g. defaults for externalClusters) they should live in the same recipe so make generate remains the single source of truth. That keeps the build deterministic and avoids hidden drift.

packages/extra/seaweedfs/templates/cm.yaml (1)

3-5: Set namespace explicitly for clarity

Although Helm will default resources into the release namespace, adding it explicitly avoids surprises when the chart is templated without --namespace or in GitOps diff tools.

 metadata:
   name: {{ .Release.Name }}-deployed-topology
+  namespace: {{ .Release.Namespace }}

packages/extra/seaweedfs/templates/external-clients/deploy.yaml (1)

87-88: Pin the sidecar image to a version tag rather than latest

Using :latest@sha256… hampers reproducibility and cache efficiency. Prefer a semver tag (e.g. v0.4.1) together with the digest.

- image: ghcr.io/cozystack/cozystack/objectstorage-controller:latest@sha256:…
+ image: ghcr.io/cozystack/cozystack/objectstorage-controller:v0.4.1@sha256:…

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/extra/seaweedfs/templates/external/cosi-cluster-role.yaml (1)

36-47: Secret write verbs are still present – high-risk cluster-wide exposure

The ClusterRole continues to grant create / update / delete / patch on every Secret in the cluster.
That is an unnecessary escalation for an object-storage provisioner and was already called out in the earlier review.

   resources:
     - "secrets"
     - "events"
   verbs:
-    - "get"
-    - "list"
-    - "watch"
-    - "update"
-    - "create"
-    - "delete"
-    - "patch"
+    - "get"
+    - "list"
+    - "watch"

If write access to Secrets is absolutely required, move those verbs to a namespaced Role scoped only to the namespaces where secret mutation is unavoidable.

packages/extra/seaweedfs/templates/external-clients/deploy.yaml (4)

1-3: lookup nil-safety missing – template panics when the Namespace object is absent

lookup returns nil when rendering against an API server (or helm template) without the Namespace.
Accessing .metadata.annotations on nil causes a hard failure.

-{{- $ingress := index $myNS.metadata.annotations "namespace.cozystack.io/ingress" }}
-{{- $host := index $myNS.metadata.annotations "namespace.cozystack.io/host" }}
+{{- $ingress := "" }}
+{{- $host := "" }}
+{{- if $myNS }}
+  {{- $ingress = index $myNS.metadata.annotations "namespace.cozystack.io/ingress" | default "" }}
+  {{- $host    = index $myNS.metadata.annotations "namespace.cozystack.io/host"    | default "" }}
+{{- end }}

---

34-35: Pods will run under the default ServiceAccount – RBAC rules won’t apply

serviceAccountName is missing. The containers therefore inherit the default SA and lack the ClusterRoleBinding you created.

 spec:
+  serviceAccountName: {{ $.Release.Namespace }}-seaweedfs-objectstorage-provisioner
   containers:

---

95-97: POD_NAMESPACE env var should reflect the running pod’s namespace, not the remote client

Hard-coding $client.namespace misleads components that rely on this variable (leader-election, event creation, metrics labels).

-          value: {{ $client.namespace }}
+          valueFrom:
+            fieldRef:
+              fieldPath: metadata.namespace

---

108-109: socket volume lacks a source – manifest will be rejected by the API server

name alone is insufficient; add emptyDir: {} (or another source) to make it a valid volume spec.

 - name: socket
+  emptyDir: {}

packages/extra/seaweedfs/templates/external/cosi-cluster-role.yaml (1)

20-35: Indentation does not match YAML spec – template renders but fails helm lint

The verbs arrays are indented with 4 spaces beneath their parent key, while the preceding keys are at 2-space depth.
Helm will happily render the manifest, but helm lint / yamllint report errors and downstream automation may reject the file.

-  verbs:
-      - "get"
-      - "list"
+  verbs:
+    - "get"
+    - "list"

Apply the same 2-space indentation to all verbs: blocks for consistency.

Also applies to: 40-47

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/extra/seaweedfs/values.yaml (1)

35-54: Example block leaks a full JWT & fails linters – replace with a placeholder

Keeping a real-looking token in the repo triggers secret-scanners (see Gitleaks) and the trailing blanks on several comment lines break yamllint.
Sanitise the example and strip whitespace.

 ##     token: "eyJhbGciOiJSUzI1NiIsImtpZCI6IlJMaEMyMTVvdDVVSENqZ1EySEFtVUhobnp2Mi02RXJ4bWM0bDJVenJmYzgifQ...
+##     # token: "<REDACTED>"

Afterwards run make lint – it should go green.

packages/extra/seaweedfs/templates/external-clients/deploy.yaml (3)

1-3: Guard against nil lookup to prevent template panic

Same nil-deref risk as noted elsewhere. Wrap the annotation extraction in a check (see previous diff).

---

107-108: socket volume lacks a source – manifest rejected by API server

Add an emptyDir definition.

 - name: socket
+  emptyDir: {}

---

90-106: Deployment runs with default ServiceAccount – RBAC will break

serviceAccountName is missing; the driver & sidecar inherit the default SA in the remote namespace, which has no perms.

 spec:
+  serviceAccountName: {{ $.Release.Name }}-objectstorage-provisioner
   containers:

packages/extra/seaweedfs/values.yaml (1)

6-9: Consider reflecting the new External mode in the default / docs

topology now supports External, yet the default and surrounding docs still emphasise Simple.
If External is expected to be the common path, update the default and/or add a note explaining when to use each mode.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro