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

Add sprig functions #5593

Merged
merged 2 commits into from
Oct 23, 2024
Merged

Add sprig functions #5593

merged 2 commits into from
Oct 23, 2024

Conversation

kfox1111
Copy link
Contributor

Fixes: #5575

Comment on lines 73 to 80
"deepCopy",
"mustDeepCopy",
"typeOf",
"typeIs",
"typeIsLike",
"kindOf",
"kindIs",
"deepEqual",
Copy link
Member

Choose a reason for hiding this comment

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

I'd probably leave out the reflection funcs for now?

sprigMap := sprig.TxtFuncMap()
ourMap := make(template.FuncMap)
for _, f := range funcList {
ourMap[f] = sprigMap[f]
Copy link
Member

Choose a reason for hiding this comment

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

We should fail if the expected function does not exist in sprigMap (e.g. we had a typo or a function was removed)

Comment on lines 158 to 160
sprigMap := sprig.TxtFuncMap()
ourMap := make(template.FuncMap)
for _, f := range funcList {
Copy link
Member

Choose a reason for hiding this comment

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

I'd probably construct ourMap once at package initialization time?

)

var funcList = [...]string{
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var funcList = [...]string{
var funcList = []string{

"randAscii",
"randNumeric",
"swapcase",
"shuffle",
Copy link
Member

Choose a reason for hiding this comment

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

🤔 I'd probably leave this one out?

Comment on lines 65 to 67
"toJson",
"toPrettyJson",
"toRawJson",
Copy link
Member

Choose a reason for hiding this comment

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

Can't imagine wanting to convert something to JSON inside of a path template...

Comment on lines 69 to 71
"mustToJson",
"mustToPrettyJson",
"mustToRawJson",
Copy link
Member

Choose a reason for hiding this comment

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

Can't imagine wanting to convert something to JSON inside of a path template...

@azdagron
Copy link
Member

Hmm, looking at the failing windows integration test, I think we're falling victim to package init order....

Fixes: spiffe#5575

Signed-off-by: Kevin Fox <[email protected]>
@kfox1111
Copy link
Contributor Author

The string its failing on (sha512sum) isn't in the codebase anywhere anymore.... It was in an earlier version of the pr. Could the vm be persisting a bad pr somehow?

@azdagron azdagron added this to the 1.11.1 milestone Oct 23, 2024
@azdagron azdagron merged commit 915b0e7 into spiffe:main Oct 23, 2024
34 checks passed
@kfox1111 kfox1111 deleted the sprig branch October 23, 2024 21:22
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.

[spire-server] go template functions (sprig)
2 participants