Skip to content
Merged
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
38 changes: 36 additions & 2 deletions flux/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,15 @@ spec:
- command:
- /bin/sh
- -c
- mkdir -p /build/plugins && cp -r /plugins/* /build/plugins/
- mkdir -p /build/plugins && cp -r /plugins/* /build/plugins/ && chown -R 100:101 /build
Copy link
Contributor

@illume illume Jul 3, 2025

Choose a reason for hiding this comment

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

Oh... I just realised the 100:101 here is the headlamp user and group from the alpine headlamp Dockerfile container?

Can we use chown -R headlamp:headlamp /build here instead?

How about a comment explaining why the chown is necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh... I just realised the 100:101 here is the headlamp user and group from the alpine headlamp Dockerfile container?

Yes
https://github.com/headlamp-k8s/plugins/blob/main/Dockerfile#L45
Actually we can improve also this line, to define exactly the number of the group to be more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we use chown -R headlamp:headlamp /build here instead?

yes, but it's not a true way, because from image to image the number of the headlamp group could be diff.
It's a better to update the Dockerfile to set up a static number, smth like:

# Create a group with specific GID and user with specific UID
RUN addgroup -g 101 -S headlamp && \
    adduser -u 100 -S -G headlamp headlamp

I can update it, WDYT ?

Copy link
Contributor

@illume illume Jul 7, 2025

Choose a reason for hiding this comment

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

Thanks. I understand I think... so if someone uses headlamp:headlamp in say their own custom Debian image maybe it's different from an alpine image?

Yeah, I think it's a good change to make.

image: ghcr.io/headlamp-k8s/headlamp-plugin-flux:latest
imagePullPolicy: Always
name: headlamp-plugins
securityContext:
Copy link
Contributor

@illume illume Jul 3, 2025

Choose a reason for hiding this comment

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

This part here is required because of the chown used above? Or is it for something else?

Could you please add a comment to there explaining why it's necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The securityContext overwrite the USER headlamp non root user.
Without it, we don't have enough permissions to do the dirs with non headlamp owner.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, that makes sense now. Thanks.

Could you please add a comment about that in there for future readers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, that makes sense now. Thanks.

Could you please add a comment about that in there for future readers?

I can, it's not a big deal.
But, are you sure if it's necessary? Because it's a pretty simple setting. Moreover, we can't annotate every line. We should do it, only if it's some exception or TODO or workaround.

WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Umm i don't understand this change we already did chwon above so we would not want to run this as root right??

maybe this is what we need here

securityContext:
  runAsNonRoot: true
  runAsUser: 100
  runAsGroup: 101
  privileged: false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's true. What is the user who run this cmd?
Without it, you will fail.
Btw, you can try to run without these changes to check it.

Copy link
Contributor Author

@kksudo kksudo Jul 7, 2025

Choose a reason for hiding this comment

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

Umm i don't understand this change we already did chwon above so we would not want to run this as root right??

maybe this is what we need here

securityContext:
  runAsNonRoot: true
  runAsUser: 100
  runAsGroup: 101
  privileged: false

@ashu8912
Again, did you check the root cause of these changes here ?

image

runAsNonRoot: false
privileged: false
runAsUser: 0
runAsGroup: 0
volumeMounts:
- mountPath: /build/plugins
name: headlamp-plugins
Expand All @@ -85,5 +90,34 @@ spec:
volumes:
- name: headlamp-plugins
persistentVolumeClaim:
claimName: headlamp
claimName: headlamp # The name of the Helm release
```

As alternative, you can also use the Use EmptyDir (Ephemeral Shared Volume) to pass files from the init containers to the main container.

```yaml
config:
pluginsDir: /build/plugins
initContainers:
- command:
- /bin/sh
- -c
- mkdir -p /build/plugins && cp -r /plugins/* /build/plugins/ && chown -R 100:101 /build
image: ghcr.io/headlamp-k8s/headlamp-plugin-flux:latest
imagePullPolicy: Always
name: headlamp-plugins
securityContext:
runAsNonRoot: false
privileged: false
runAsUser: 0
runAsGroup: 0
volumeMounts:
- mountPath: /build/plugins
name: headlamp-plugins
volumeMounts:
- mountPath: /build/plugins
name: headlamp-plugins
volumes:
- name: headlamp-plugins
emptyDir: {}
```
Loading