Skip to content

Add Windows instance flag support for named pipes#6977

Open
Devansh1093 wants to merge 5 commits into
spiffe:mainfrom
Devansh1093:feat/windows-instance-flag
Open

Add Windows instance flag support for named pipes#6977
Devansh1093 wants to merge 5 commits into
spiffe:mainfrom
Devansh1093:feat/windows-instance-flag

Conversation

@Devansh1093

@Devansh1093 Devansh1093 commented May 21, 2026

Copy link
Copy Markdown

Pull Request check list

  • Commit conforms to CONTRIBUTING.md?
  • Proper tests/regressions included?
  • Documentation updated?

Affected functionality

Windows named pipe address resolution for SPIRE server CLI utilities.

Description of change

Adds support for the instance flag in util_windows.go to align Windows behavior with the existing POSIX implementation.

This change enables named pipe paths to be resolved using template environment variables similarly to the POSIX socket template flow.

The implementation intentionally keeps the %i placeholder format consistent with the POSIX implementation to preserve cross-platform behavior and avoid introducing a separate Windows-specific template syntax.

Validation performed:

  • go test ./...
  • GOOS=windows go build ./...

Which issue this PR fixes

Related to #6937

Signed-off-by: Devansh1093 <dr30031102@gmail.com>

@sorindumitru sorindumitru left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @Devansh1093 for the changes. I think spire-agent would also require similar changes to support the instance environment variable.

@Devansh1093 Devansh1093 force-pushed the feat/windows-instance-flag branch from d167c8f to c0c8e75 Compare June 2, 2026 09:27
Copilot AI review requested due to automatic review settings June 2, 2026 09:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@Devansh1093 Devansh1093 force-pushed the feat/windows-instance-flag branch from c0c8e75 to 75ff5f2 Compare June 2, 2026 09:51
Signed-off-by: Devansh1093 <dr30031102@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 25 comments.

Comment on lines 5 to 16
import (
"context"
"flag"
"net"
"strings"

"fmt"
"os"

"github.com/Microsoft/go-winio"
"github.com/spiffe/spire/pkg/common/namedpipe"
)
Comment on lines 23 to 26
func (a *Adapter) addOSFlags(flags *flag.FlagSet) {
flags.StringVar(&a.namedPipeName, "namedPipeName", DefaultNamedPipeName, "Pipe name of the SPIRE Server API named pipe")
flags.StringVar(&a.instance, "instance", "", "Instance name to substitute into socket templates (env SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE).")
}
Comment on lines 35 to +66
func (a *Adapter) getGRPCAddr() (string, error) {
if a.namedPipeName == "" {
a.namedPipeName = DefaultNamedPipeName
tpl := os.Getenv("SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE")
pipe := os.Getenv("SPIRE_SERVER_PRIVATE_SOCKET")

if a.instance != "" {
if tpl == "" {
return "", fmt.Errorf(
"you must define %s to use the instance flag",
"SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE",
)
}

if !strings.Contains(tpl, "%i") {
return "", fmt.Errorf(
"failed to find %%i in %s",
"SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE",
)
}
}

namedPipeName := DefaultNamedPipeName

switch {
case a.namedPipeName != DefaultNamedPipeName:
namedPipeName = a.namedPipeName

case a.instance != "":
namedPipeName = strings.ReplaceAll(tpl, "%i", a.instance)

case pipe != "":
namedPipeName = pipe
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, it would be good to have a test for this

Comment on lines 6 to 12
setUsage = `Usage of logger set:
-instance string
Instance name to substitute into socket templates (env SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE).

-level string
The new log level, one of (panic, fatal, error, warn, info, debug, trace)
-namedPipeName string
Comment on lines 6 to 12
getUsage = `Usage of logger get:
-instance string
Instance name to substitute into socket templates (env SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE).
-namedPipeName string
-namedPipeName string
Pipe name of the SPIRE Server API named pipe (default "\\spire-server\\private\\api")
-output value
`
evictUsage = `Usage of agent evict:
-instance string
Instance name to substitute into socket templates (env SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE).
-matchSelectorsOn string
The match mode used when filtering by selectors. Options: exact, any, superset and subset (default "superset")
-instance string
Instance name to substitute into socket templates (env SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE).
`
showUsage = `Usage of agent show:
-instance string
Instance name to substitute into socket templates (env SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE).
-jwtSVIDTTL int
The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry.
-instance string
Instance name to substitute into socket templates (env SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE).
-matchSelectorsOn string
The match mode used when filtering by selectors. Options: exact, any, superset and subset (default "superset")
-instance string
Instance name to substitute into socket templates (env SPIRE_SERVER_PRIVATE_SOCKET_TEMPLATE).
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Devansh Roy <dr30031102@gmail.com>
@Devansh1093

Copy link
Copy Markdown
Author

Hi @sorindumitru, I am a bit confused here. Do I need to make changes in my code suggested by copilot? . It would be helpful if you could tell me.

@sorindumitru

Copy link
Copy Markdown
Member

Hi @sorindumitru, I am a bit confused here. Do I need to make changes in my code suggested by copilot? . It would be helpful if you could tell me.

Hi @Devansh1093, yes, it would be good to go through those and see which make sense to address or not. I'll also go through them when reviewing the code.

@sorindumitru

Copy link
Copy Markdown
Member
  • go test ./...
  • GOOS=windows go build ./...

Did you run the go test ./... command on a Windows machine? It won't run the windows specific tests on other operating systems. There are some tests failures now which need addressing.

@Devansh1093

Copy link
Copy Markdown
Author

I ran the tests on linux system.

@sorindumitru

Copy link
Copy Markdown
Member

I ran the tests on linux system.

You'll have to find a way to run them on windows, to test it out. For example via a VM, microsoft has ISOs available for evaluation purposes.

@Devansh1093

Copy link
Copy Markdown
Author

I am not able to run them on virtual machines. I tried multiple times.

@sorindumitru

Copy link
Copy Markdown
Member

I am not able to run them on virtual machines. I tried multiple times.

Does something like go test ./... not work, if you have Go installed and in the path? Otherwise you might have to use the full path to the go binary.

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.

3 participants