Skip to content

Fix infinite recursion in inline-certificate config entry #16276

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.

Sign up for GitHub

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

Merged
nathancoleman merged 4 commits into from
Feb 15, 2023
Merged

Fix infinite recursion in inline-certificate config entry #16276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ea4dd2b
Fix infinite recursion on InlineCertificateConfigEntry
nathancoleman Feb 14, 2023
637d13a
Add tests for inline-certificate
nathancoleman Feb 15, 2023
18f19ce
Add alias for private key field on inline-certificate
nathancoleman Feb 15, 2023
3e6807b
Use valid certificate + private key for inline-certificate tests
nathancoleman Feb 15, 2023
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
38 changes: 7 additions & 31 deletions agent/structs/config_entry_inline_certificate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,14 @@ type InlineCertificateConfigEntry struct {
RaftIndex
Copy link
Member Author

@nathancoleman nathancoleman Feb 15, 2023

Choose a reason for hiding this comment

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

The changes in this file are just to have these funcs match the format/feel of matching funcs across other config entry types

}

func (e *InlineCertificateConfigEntry) GetKind() string {
return InlineCertificate
}

func (e *InlineCertificateConfigEntry) GetName() string {
return e.Name
}

func (e *InlineCertificateConfigEntry) Normalize() error {
return nil
func (e *InlineCertificateConfigEntry) GetKind() string { return InlineCertificate }
func (e *InlineCertificateConfigEntry) GetName() string { return e.Name }
func (e *InlineCertificateConfigEntry) Normalize() error { return nil }
func (e *InlineCertificateConfigEntry) GetMeta() map[string]string { return e.Meta }
func (e *InlineCertificateConfigEntry) GetEnterpriseMeta() *acl.EnterpriseMeta {
Copy link
Member

@jm96441n jm96441n Feb 15, 2023

Choose a reason for hiding this comment

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

this is the most minor thing, but could we put a newline above this function or move the above opening braces to be inline with this one? totally not necessary to do and purely aesthetic 😹

Copy link
Member Author

@nathancoleman nathancoleman Feb 15, 2023

Choose a reason for hiding this comment

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

I tried moving the braces to be inline, but gofmt wants it this way. Given the flakiness of tests and that this is how our other config entries are today, Imma leave it like this for now

return &e.EnterpriseMeta
}
func (e *InlineCertificateConfigEntry) GetRaftIndex() *RaftIndex { return &e.RaftIndex }

func (e *InlineCertificateConfigEntry) Validate() error {
privateKeyBlock, _ := pem.Decode([]byte(e.PrivateKey))
Expand Down Expand Up @@ -78,24 +75,3 @@ func (e *InlineCertificateConfigEntry) CanWrite(authz acl.Authorizer) error {
e.FillAuthzContext(&authzContext)
return authz.ToAllowAuthorizer().MeshWriteAllowed(&authzContext)
}

func (e *InlineCertificateConfigEntry) GetMeta() map[string]string {
if e == nil {
return nil
}
return e.Meta
}

func (e *InlineCertificateConfigEntry) GetEnterpriseMeta() *acl.EnterpriseMeta {
if e == nil {
return nil
}
return &e.EnterpriseMeta
}

func (e *InlineCertificateConfigEntry) GetRaftIndex() *RaftIndex {
if e == nil {
return &RaftIndex{}
}
return &e.RaftIndex
}
48 changes: 8 additions & 40 deletions api/config_entry_inline_certificate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ type InlineCertificateConfigEntry struct {
// Certificate is the public certificate component of an x509 key pair encoded in raw PEM format.
Certificate string
// PrivateKey is the private key component of an x509 key pair encoded in raw PEM format.
PrivateKey string
PrivateKey string `alias:"private_key"`
Copy link
Member

@jm96441n jm96441n Feb 15, 2023

Choose a reason for hiding this comment

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

where does alias come from?

Copy link
Contributor

@andrewstucki andrewstucki Feb 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

alias is used in the HCL parser we leverage for making it so you can pass the value snake-cased rather than camelized, we leverage it all over the API package, for example:

consul/api/config_entry.go

Line 98 in 6599a9b

OutboundListenerPort int `json:",omitempty" alias:"outbound_listener_port"`

It's pretty typical that we prefer snake-casing to camelized values in the ConfigEntry HCL definitions -- i.e. private_key = ... v. privateKey = ...

Copy link
Member

@jm96441n jm96441n Feb 15, 2023

Choose a reason for hiding this comment

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

TIL! that's pretty nifty


Meta map[string]string `json:",omitempty"`

Expand All @@ -34,42 +34,10 @@ type InlineCertificateConfigEntry struct {
Namespace string `json:",omitempty"`
}

func (a *InlineCertificateConfigEntry) GetKind() string {
return InlineCertificate
}

func (a *InlineCertificateConfigEntry) GetName() string {
if a != nil {
return ""
}
return a.Name
}

func (a *InlineCertificateConfigEntry) GetPartition() string {
if a != nil {
return ""
}
return a.Partition
}

func (a *InlineCertificateConfigEntry) GetNamespace() string {
if a != nil {
return ""
}
return a.GetNamespace()
Copy link
Member Author

@nathancoleman nathancoleman Feb 15, 2023

Choose a reason for hiding this comment

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

Infinite recursion
image

}

func (a *InlineCertificateConfigEntry) GetMeta() map[string]string {
if a != nil {
return nil
}
return a.GetMeta()
Copy link
Member Author

@nathancoleman nathancoleman Feb 15, 2023

Choose a reason for hiding this comment

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

Infinite recursion
image

}

func (a *InlineCertificateConfigEntry) GetCreateIndex() uint64 {
return a.CreateIndex
}

func (a *InlineCertificateConfigEntry) GetModifyIndex() uint64 {
return a.ModifyIndex
}
func (a *InlineCertificateConfigEntry) GetKind() string { return InlineCertificate }
func (a *InlineCertificateConfigEntry) GetName() string { return a.Name }
func (a *InlineCertificateConfigEntry) GetPartition() string { return a.Partition }
func (a *InlineCertificateConfigEntry) GetNamespace() string { return a.Namespace }
func (a *InlineCertificateConfigEntry) GetMeta() map[string]string { return a.Meta }
func (a *InlineCertificateConfigEntry) GetCreateIndex() uint64 { return a.CreateIndex }
func (a *InlineCertificateConfigEntry) GetModifyIndex() uint64 { return a.ModifyIndex }
122 changes: 122 additions & 0 deletions api/config_entry_inline_certificate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package api

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

const (
// generated via openssl req -x509 -sha256 -days 1825 -newkey rsa:2048 -keyout private.key -out certificate.crt
validPrivateKey = `-----BEGIN RSA PRIVATE KEY-----
MIIEowIBAAKCAQEAx95Opa6t4lGEpiTUogEBptqOdam2ch4BHQGhNhX/MrDwwuZQ
httBwMfngQ/wd9NmYEPAwj0dumUoAITIq6i2jQlhqTodElkbsd5vWY8R/bxJWQSo
NvVE12TlzECxGpJEiHt4W0r8pGffk+rvpljiUyCfnT1kGF3znOSjK1hRMTn6RKWC
yYaBvXQiB4SGilfLgJcEpOJKtISIxmZ+S409g9X5VU88/Bmmrz4cMyxce86Kc2ug
5/MOv0CjWDJwlrv8njneV2zvraQ61DDwQftrXOvuCbO5IBRHMOBHiHTZ4rtGuhMa
Ir21V4vb6n8c4YzXiFvhUYcyX7rltGZzVd+WmQIDAQABAoIBACYvceUzp2MK4gYA
GWPOP2uKbBdM0l+hHeNV0WAM+dHMfmMuL4pkT36ucqt0ySOLjw6rQyOZG5nmA6t9
sv0g4ae2eCMlyDIeNi1Yavu4Wt6YX4cTXbQKThm83C6W2X9THKbauBbxD621bsDK
7PhiGPN60yPue7YwFQAPqqD4YaK+s22HFIzk9gwM/rkvAUNwRv7SyHMiFe4Igc1C
Eev7iHWzvj5Heoz6XfF+XNF9DU+TieSUAdjd56VyUb8XL4+uBTOhHwLiXvAmfaMR
HvpcxeKnYZusS6NaOxcUHiJnsLNWrxmJj9WEGgQzuLxcLjTe4vVmELVZD8t3QUKj
PAxu8tUCgYEA7KIWVn9dfVpokReorFym+J8FzLwSktP9RZYEMonJo00i8aii3K9s
u/aSwRWQSCzmON1ZcxZzWhwQF9usz6kGCk//9+4hlVW90GtNK0RD+j7sp4aT2JI8
9eLEjTG+xSXa7XWe98QncjjL9lu/yrRncSTxHs13q/XP198nn2aYuQ8CgYEA2Dnt
sRBzv0fFEvzzFv7G/5f85mouN38TUYvxNRTjBLCXl9DeKjDkOVZ2b6qlfQnYXIru
H+W+v+AZEb6fySXc8FRab7lkgTMrwE+aeI4rkW7asVwtclv01QJ5wMnyT84AgDD/
Dgt/RThFaHgtU9TW5GOZveL+l9fVPn7vKFdTJdcCgYEArJ99zjHxwJ1whNAOk1av
09UmRPm6TvRo4heTDk8oEoIWCNatoHI0z1YMLuENNSnT9Q280FFDayvnrY/qnD7A
kktT/sjwJOG8q8trKzIMqQS4XWm2dxoPcIyyOBJfCbEY6XuRsUuePxwh5qF942EB
yS9a2s6nC4Ix0lgPrqAIr48CgYBgS/Q6riwOXSU8nqCYdiEkBYlhCJrKpnJxF9T1
ofa0yPzKZP/8ZEfP7VzTwHjxJehQ1qLUW9pG08P2biH1UEKEWdzo8vT6wVJT1F/k
HtTycR8+a+Hlk2SHVRHqNUYQGpuIe8mrdJ1as4Pd0d/F/P0zO9Rlh+mAsGPM8HUM
T0+9gwKBgHDoerX7NTskg0H0t8O+iSMevdxpEWp34ZYa9gHiftTQGyrRgERCa7Gj
nZPAxKb2JoWyfnu3v7G5gZ8fhDFsiOxLbZv6UZJBbUIh1MjJISpXrForDrC2QNLX
kHrHfwBFDB3KMudhQknsJzEJKCL/KmFH6o0MvsoaT9yzEl3K+ah/
-----END RSA PRIVATE KEY-----`
validCertificate = `-----BEGIN CERTIFICATE-----
MIICljCCAX4CCQCQMDsYO8FrPjANBgkqhkiG9w0BAQsFADANMQswCQYDVQQGEwJV
UzAeFw0yMjEyMjAxNzUwMjVaFw0yNzEyMTkxNzUwMjVaMA0xCzAJBgNVBAYTAlVT
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAx95Opa6t4lGEpiTUogEB
ptqOdam2ch4BHQGhNhX/MrDwwuZQhttBwMfngQ/wd9NmYEPAwj0dumUoAITIq6i2
jQlhqTodElkbsd5vWY8R/bxJWQSoNvVE12TlzECxGpJEiHt4W0r8pGffk+rvplji
UyCfnT1kGF3znOSjK1hRMTn6RKWCyYaBvXQiB4SGilfLgJcEpOJKtISIxmZ+S409
g9X5VU88/Bmmrz4cMyxce86Kc2ug5/MOv0CjWDJwlrv8njneV2zvraQ61DDwQftr
XOvuCbO5IBRHMOBHiHTZ4rtGuhMaIr21V4vb6n8c4YzXiFvhUYcyX7rltGZzVd+W
mQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQBfCqoUIdPf/HGSbOorPyZWbyizNtHJ
GL7x9cAeIYxpI5Y/WcO1o5v94lvrgm3FNfJoGKbV66+JxOge731FrfMpHplhar1Z
RahYIzNLRBTLrwadLAZkApUpZvB8qDK4knsTWFYujNsylCww2A6ajzIMFNU4GkUK
NtyHRuD+KYRmjXtyX1yHNqfGN3vOQmwavHq2R8wHYuBSc6LAHHV9vG+j0VsgMELO
qwxn8SmLkSKbf2+MsQVzLCXXN5u+D8Yv+4py+oKP4EQ5aFZuDEx+r/G/31rTthww
AAJAMaoXmoYVdgXV+CPuBb2M4XCpuzLu3bcA2PXm5ipSyIgntMKwXV7r
-----END CERTIFICATE-----`
)

func TestAPI_ConfigEntries_InlineCertificate(t *testing.T) {
Copy link
Member Author

@nathancoleman nathancoleman Feb 15, 2023

Choose a reason for hiding this comment

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

New tests for this config entry kind. Just exercises CRUD ops

t.Parallel()
c, s := makeClient(t)
defer s.Stop()

configEntries := c.ConfigEntries()

cert1 := &InlineCertificateConfigEntry{
Kind: InlineCertificate,
Name: "cert1",
Meta: map[string]string{"foo": "bar"},
Certificate: validCertificate,
PrivateKey: validPrivateKey,
}

// set it
_, wm, err := configEntries.Set(cert1, nil)
require.NoError(t, err)
assert.NotNil(t, wm)

// get it
entry, qm, err := configEntries.Get(InlineCertificate, "cert1", nil)
require.NoError(t, err)
require.NotNil(t, qm)
assert.NotEqual(t, 0, qm.RequestTime)

readCert, ok := entry.(*InlineCertificateConfigEntry)
require.True(t, ok)
assert.Equal(t, cert1.Kind, readCert.Kind)
assert.Equal(t, cert1.Name, readCert.Name)
assert.Equal(t, cert1.Meta, readCert.Meta)
assert.Equal(t, cert1.Meta, readCert.GetMeta())

// update it
cert1.Meta["bar"] = "baz"
written, wm, err := configEntries.CAS(cert1, readCert.ModifyIndex, nil)
require.NoError(t, err)
require.NotNil(t, wm)
assert.NotEqual(t, 0, wm.RequestTime)
assert.True(t, written)

// list it
entries, qm, err := configEntries.List(InlineCertificate, nil)
require.NoError(t, err)
require.NotNil(t, qm)
assert.NotEqual(t, 0, qm.RequestTime)

require.Len(t, entries, 1)
assert.Equal(t, cert1.Kind, entries[0].GetKind())
assert.Equal(t, cert1.Name, entries[0].GetName())

readCert, ok = entries[0].(*InlineCertificateConfigEntry)
require.True(t, ok)
assert.Equal(t, cert1.Certificate, readCert.Certificate)
assert.Equal(t, cert1.Meta, readCert.Meta)

// delete it
wm, err = configEntries.Delete(InlineCertificate, cert1.Name, nil)
require.NoError(t, err)
require.NotNil(t, wm)
assert.NotEqual(t, 0, wm.RequestTime)

// try to get it
_, _, err = configEntries.Get(InlineCertificate, cert1.Name, nil)
assert.Error(t, err)
}