Skip to content

Add Tornjak manager CRD#610

Open
Alan-Cha wants to merge 7 commits into
spiffe:mainfrom
Alan-Cha:2.1.0-manager
Open

Add Tornjak manager CRD#610
Alan-Cha wants to merge 7 commits into
spiffe:mainfrom
Alan-Cha:2.1.0-manager

Conversation

@Alan-Cha

Copy link
Copy Markdown
Contributor

Built on top of #597

@kfox1111

Copy link
Copy Markdown
Collaborator

This seems to have overlap with 597. Is there anything left todo since that has merged?

Comment on lines +69 to 74

> [!IMPORTANT]
> If audience is set, make sure the Tornjak backend `audience` is set correctly. You can set it using:
> `--set spire-server.tornjak.config.userManagement.audience=your-audience`
>

> [!TIP]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These sections were not rendering correctly because there's no separation between them.

@Alan-Cha

Copy link
Copy Markdown
Contributor Author

@kfox1111 I updated the branch. Should be ready for merging now!

@Alan-Cha Alan-Cha marked this pull request as ready for review July 11, 2025 17:46
classname = "{{ .Values.controllerManager.className }}"
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm....

Is the crd manager functionality disable-able? If so, is it leaving this section out?

Does it use the permissions of the k8s service account in order to manage the crds?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Back when it was under development, Tornjak shared the same pod as the controller manager, and spire server (IIRC). Then, it inherited all of the permissions of the controller manager which is what is necessary. Has this changed?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We support running the spire-server without the controller-manager, so it may not have crd support (I have a server configured this way)... but maybe you require the controller manager to function properly....

So you probably should either add the permissions if you can work without the crd in case its disabled, or error the chart out if you expect crds to be there and the controller manager is disabled?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Alan-Cha @maia-iyer do you have time to address the comment from @kfox1111

Alan-Cha added 7 commits July 30, 2025 10:09
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
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

Successfully merging this pull request may close these issues.

4 participants