Skip to content

Commit

Permalink
Fix EnterpriseSearch upgrade with TLS disabled (#6224)
Browse files Browse the repository at this point in the history
During an EnterpriseSearch upgrade, ECK calls its API to set the read-only 
mode setting and we missed adjusting the HTTP client used for this call 
based on whether or not TLS is enabled.

This commit correctly creates the HTTP client to call the EnterpriseSearch 
API when TLS is disabled at the EnterpriseSearch resource level.

The existing e2e test TestEnterpriseSearchTLSDisabled is converted to TestEnterpriseSearchTLSDisabledVersionUpgradeToLatest8x to cover this bug.
  • Loading branch information
thbkrkr authored Dec 20, 2022
1 parent 3522b44 commit 984e278
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 18 deletions.
10 changes: 7 additions & 3 deletions pkg/controller/enterprisesearch/version_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,13 @@ func (r *VersionUpgrade) setReadOnlyMode(ctx context.Context, enabled bool) erro
httpClient := r.httpClient
if httpClient == nil {
// build an HTTP client to reach the Enterprise Search service
tlsCerts, err := r.retrieveTLSCerts()
if err != nil {
return err
var tlsCerts []*x509.Certificate
if r.ent.Spec.HTTP.TLS.Enabled() {
var err error
tlsCerts, err = r.retrieveTLSCerts()
if err != nil {
return err
}
}
httpClient = apmhttp.WrapClient(
commonhttp.Client(r.dialer, tlsCerts, 0),
Expand Down
41 changes: 26 additions & 15 deletions test/e2e/ent/ent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,6 @@ func TestEnterpriseSearchCrossNSAssociation(t *testing.T) {
test.Sequence(nil, test.EmptySteps, esBuilder, entBuilder).RunSequential(t)
}

func TestEnterpriseSearchTLSDisabled(t *testing.T) {
name := "test-ent-tls-disabled"

esBuilder := elasticsearch.NewBuilder(name).
WithESMasterDataNodes(1, elasticsearch.DefaultResources).
WithRestrictedSecurityContext()
entBuilder := enterprisesearch.NewBuilder(name).
WithElasticsearchRef(esBuilder.Ref()).
WithNodeCount(1).
WithTLSDisabled(true).
WithRestrictedSecurityContext()

test.Sequence(nil, test.EmptySteps, esBuilder, entBuilder).RunSequential(t)
}

func TestEnterpriseSearchVersionUpgradeToLatest7x(t *testing.T) {
srcVersion := test.Ctx().ElasticStackVersion
dstVersion := test.LatestReleasedVersion7x
Expand Down Expand Up @@ -97,3 +82,29 @@ func TestEnterpriseSearchVersionUpgradeToLatest8x(t *testing.T) {
// runs fine in the new version: it would fail to run if read-only mode wasn't toggled.
test.RunMutations(t, []test.Builder{es, ent}, []test.Builder{esUpgraded, entUpgraded})
}

func TestEnterpriseSearchTLSDisabledVersionUpgradeToLatest8x(t *testing.T) {
srcVersion, dstVersion := test.GetUpgradePathTo8x(test.Ctx().ElasticStackVersion)

test.SkipInvalidUpgrade(t, srcVersion, dstVersion)

name := "test-ent-notls-version-upgrade-8x"
es := elasticsearch.NewBuilder(name).
WithESMasterDataNodes(1, elasticsearch.DefaultResources).
WithVersion(srcVersion)

ent := enterprisesearch.NewBuilder(name).
WithElasticsearchRef(es.Ref()).
WithNodeCount(2).
WithVersion(srcVersion).
WithTLSDisabled(true).
WithRestrictedSecurityContext()

esUpgraded := es.WithVersion(dstVersion).WithMutatedFrom(&es)
entUpgraded := ent.WithVersion(dstVersion).WithMutatedFrom(&ent)

// During the version upgrade, the operator will toggle Enterprise Search read-only mode.
// We don't verify this behaviour here. Instead, we just check Enterprise Search eventually
// runs fine in the new version: it would fail to run if read-only mode wasn't toggled.
test.RunMutations(t, []test.Builder{es, ent}, []test.Builder{esUpgraded, entUpgraded})
}

0 comments on commit 984e278

Please sign in to comment.