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

Federation domain issuer must be https url #2167

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
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
18 changes: 0 additions & 18 deletions apis/concierge/config/v1alpha1/types_credentialissuer.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -161,24 +161,6 @@ type ImpersonationProxyServiceSpec struct {
type CredentialIssuerStatus struct {
// List of integration strategies that were attempted by Pinniped.
Strategies []CredentialIssuerStrategy `json:"strategies"`

// Information needed to form a valid Pinniped-based kubeconfig using this credential issuer.
// This field is deprecated and will be removed in a future version.
// +optional
KubeConfigInfo *CredentialIssuerKubeConfigInfo `json:"kubeConfigInfo,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

How confident are you that nobody is using this field? It has been deprecated almost from the start, but did our known clients all stop using it?

Also, did you test what happens on an upgrade when this field disappears from the CRD?

Copy link
Member

Choose a reason for hiding this comment

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

By the way, I believe that this was first marked deprecated in v0.7.0.

Found by using git tag --contains 60f92d5fe2f7c5da632b1639e6fd699c0e22d122.

}

// CredentialIssuerKubeConfigInfo provides the information needed to form a valid Pinniped-based kubeconfig using this credential issuer.
// This type is deprecated and will be removed in a future version.
type CredentialIssuerKubeConfigInfo struct {
// The K8s API server URL.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Pattern=`^https://|^http://`
Server string `json:"server"`

// The K8s API server CA bundle.
// +kubebuilder:validation:MinLength=1
CertificateAuthorityData string `json:"certificateAuthorityData"`
}

// CredentialIssuerStrategy describes the status of an integration strategy that was attempted by Pinniped.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ type FederationDomainSpec struct {
// See
// https://openid.net/specs/openid-connect-discovery-1_0.html#rfc.section.3 for more information.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:XValidation:message="issuer must be an HTTPS URL",rule="isURL(self) && url(self).getScheme() == 'https'"
Issuer string `json:"issuer"`

// TLS specifies a secret which will contain Transport Layer Security (TLS) configuration for the FederationDomain.
Expand Down
17 changes: 3 additions & 14 deletions cmd/pinniped/cmd/kubeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,26 +540,15 @@ func getConciergeFrontend(credentialIssuer *conciergeconfigv1alpha1.CredentialIs
continue
}

// Backfill the .status.strategies[].frontend field from .status.kubeConfigInfo for backwards compatibility.
if strategy.Type == conciergeconfigv1alpha1.KubeClusterSigningCertificateStrategyType && strategy.Frontend == nil && credentialIssuer.Status.KubeConfigInfo != nil {
strategy = *strategy.DeepCopy()
strategy.Frontend = &conciergeconfigv1alpha1.CredentialIssuerFrontend{
Type: conciergeconfigv1alpha1.TokenCredentialRequestAPIFrontendType,
TokenCredentialRequestAPIInfo: &conciergeconfigv1alpha1.TokenCredentialRequestAPIInfo{
Server: credentialIssuer.Status.KubeConfigInfo.Server,
CertificateAuthorityData: credentialIssuer.Status.KubeConfigInfo.CertificateAuthorityData,
},
}
}

// If the strategy frontend is still nil, skip.
// If the strategy frontend is nil, skip.
if strategy.Frontend == nil {
continue
}

// Skip any unknown frontend types.
switch strategy.Frontend.Type {
case conciergeconfigv1alpha1.TokenCredentialRequestAPIFrontendType, conciergeconfigv1alpha1.ImpersonationProxyFrontendType:
case conciergeconfigv1alpha1.TokenCredentialRequestAPIFrontendType,
conciergeconfigv1alpha1.ImpersonationProxyFrontendType:
default:
continue
}
Expand Down
12 changes: 6 additions & 6 deletions cmd/pinniped/cmd/kubeconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -613,18 +613,18 @@ func TestGetKubeconfig(t *testing.T) {
&conciergeconfigv1alpha1.CredentialIssuer{
ObjectMeta: metav1.ObjectMeta{Name: "test-credential-issuer"},
Status: conciergeconfigv1alpha1.CredentialIssuerStatus{
KubeConfigInfo: &conciergeconfigv1alpha1.CredentialIssuerKubeConfigInfo{
Server: "https://concierge-endpoint",
CertificateAuthorityData: "ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ==",
},
Strategies: []conciergeconfigv1alpha1.CredentialIssuerStrategy{{
Type: conciergeconfigv1alpha1.KubeClusterSigningCertificateStrategyType,
Status: conciergeconfigv1alpha1.SuccessStrategyStatus,
Reason: conciergeconfigv1alpha1.FetchedKeyStrategyReason,
Message: "Successfully fetched key",
LastUpdateTime: metav1.Now(),
// Simulate a previous version of CredentialIssuer that's missing this Frontend field.
Frontend: nil,
Frontend: &conciergeconfigv1alpha1.CredentialIssuerFrontend{
Type: conciergeconfigv1alpha1.TokenCredentialRequestAPIFrontendType,
TokenCredentialRequestAPIInfo: &conciergeconfigv1alpha1.TokenCredentialRequestAPIInfo{
Server: "https://concierge-endpoint.example.com",
},
},
}},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,24 +134,6 @@ spec:
status:
description: CredentialIssuerStatus describes the status of the Concierge.
properties:
kubeConfigInfo:
description: |-
Information needed to form a valid Pinniped-based kubeconfig using this credential issuer.
This field is deprecated and will be removed in a future version.
properties:
certificateAuthorityData:
description: The K8s API server CA bundle.
minLength: 1
type: string
server:
description: The K8s API server URL.
minLength: 1
pattern: ^https://|^http://
type: string
required:
- certificateAuthorityData
- server
type: object
strategies:
description: List of integration strategies that were attempted by
Pinniped.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,9 @@ spec:
https://openid.net/specs/openid-connect-discovery-1_0.html#rfc.section.3 for more information.
minLength: 1
type: string
x-kubernetes-validations:
- message: issuer must be an HTTPS URL
rule: isURL(self) && url(self).getScheme() == 'https'
tls:
description: TLS specifies a secret which will contain Transport Layer
Security (TLS) configuration for the FederationDomain.
Expand Down
21 changes: 0 additions & 21 deletions generated/1.25/README.adoc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 0 additions & 21 deletions generated/1.26/README.adoc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading