Skip to content

Add more specific host checks to gh attestation commands - #8948

Closed
malancas wants to merge 10 commits into
cli:trunkfrom
malancas:gh-attestation-support-ghe-hosts
Closed

malancas wants to merge 10 commits into
cli:trunkfrom
malancas:gh-attestation-support-ghe-hosts

Conversation

@malancas

@malancas malancas commented Apr 9, 2024 •

Copy link
Copy Markdown
Contributor

This adds more specific client host checking to the attestation command set. The functions used here are copied from cli/go-gh. If we are happy with this approach, I will look into whether we can export the functions in cli/go-gh and replace this custom code with those exported functions.

@williammartin

Copy link
Copy Markdown
Member

Did you copy these because they were unexported? Cause they are probably going to be exported: cli/go-gh#152

@williammartin

Copy link
Copy Markdown
Member

Yeesh sorry @malancas I totally did you wrong by not reading your PR description which I basically unintentionally answered in my last comment. FWIW I think these are also already in cli/cli here: https://github.com/cli/cli/blob/156a6974079105d3b1d517afeffc6e1a31f764f4/internal/ghinstance/host.go

Is there anything missing?

@malancas

Copy link
Copy Markdown
Contributor Author

Yeesh sorry @malancas I totally did you wrong by not reading your PR description which I basically unintentionally answered in my last comment. FWIW I think these are also already in cli/cli here: https://github.com/cli/cli/blob/156a6974079105d3b1d517afeffc6e1a31f764f4/internal/ghinstance/host.go

Is there anything missing?

No worries. I'll just wait until cli/go-gh#152 is merged and update this to use the exported functions.

@williammartin

Copy link
Copy Markdown
Member

Reiterating, these functions are already in cli/cli:

func IsEnterprise(h string) bool {
normalizedHostName := NormalizeHostname(h)
return normalizedHostName != defaultHostname && normalizedHostName != localhost
}
// IsTenancy reports whether a non-normalized host name looks like a tenancy instance.
func IsTenancy(h string) bool {
normalizedHostName := NormalizeHostname(h)
return strings.HasSuffix(normalizedHostName, "."+tenancyHost)
}

@williammartin

Copy link
Copy Markdown
Member

@phillmv I believe you had some opinions about this. Do you want this in before the release today? It doesn't appear complicated to use what is already in cli/cli.

@williammartin

Copy link
Copy Markdown
Member

#9019 succeeds this if you want it @phillmv

@malancas
malancas deleted the gh-attestation-support-ghe-hosts branch May 28, 2025 22:56
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.

2 participants