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.

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

Jump to bottom

The 'export config' subcommand should display field reference instead of values #8769

Merged
merged 1 commit into from
Oct 30, 2018
Merged

The 'export config' subcommand should display field reference instead of values #8769

merged 1 commit into from
Oct 30, 2018

Conversation

ph
Copy link
Contributor

@ph ph commented Oct 26, 2018 •
edited
Loading

Change the behavior of the export config to not display the values from
the keystore or the environment.

@ph ph added review libbeat needs_backport PR is waiting to be backported to other branches. labels Oct 26, 2018
@ph ph changed the title The 'export config' subcommand should display field reference instead of The 'export config' subcommand should display field reference instead of values Oct 26, 2018
tsg
tsg approved these changes Oct 26, 2018
View reviewed changes
Copy link
Contributor

@tsg tsg left a comment

Choose a reason for hiding this comment

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

LGTM.

@ph
Copy link
Contributor Author

ph commented Oct 26, 2018

jenkins test this please

1 similar comment
@ph
Copy link
Contributor Author

ph commented Oct 29, 2018

jenkins test this please

urso
urso suggested changes Oct 29, 2018
View reviewed changes
Copy link

@urso urso left a comment

Choose a reason for hiding this comment

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

If possible we should allow/prepare for multi-stage initialization of the beat configuration. The initialization of logging, paths, and keystore is already quite delicate.

libbeat/beat/config.go Outdated

// ConfigOpts returns ucfg config options with a resolver linked to the current keystore.
// TODO: Refactor to allow insert into the config option array without having to redefine everything
func ConfigOpts(store keystore.Keystore) []ucfg.Option {
Copy link

Choose a reason for hiding this comment

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

Let's not have these in libbeat/beat package. This package is supposed to be mostly stable providing some minimal common API for beats developers (No bc breaking changes if possible). Items with "TODO" + "Refactoring" should not land here.

Also, let's not leak the ucfg package. Libbeat is supposed to hide/wrap ucfg package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you propose to do to make sure we actually correctly initialize the resolvers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me take a look at that we pass to libbeat.

urso
urso approved these changes Oct 29, 2018
View reviewed changes
Copy link

@urso urso left a comment

Choose a reason for hiding this comment

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

LGTM

@ph ph force-pushed the fix/export-config branch from 9b274ca to 5d18917 Compare October 29, 2018 15:04
@ph
Copy link
Contributor Author

ph commented Oct 29, 2018

rebase and waiting for CI.

@ph
Copy link
Contributor Author

ph commented Oct 29, 2018

I've looked at backporting this PR to 6.2, 6.3, 6.4 all of theses versions doesn't have the possibility of passing options to libbeat when we configure it.

I have to backport #7850 and #8449 before merging that one.

@ph
Copy link
Contributor Author

ph commented Oct 29, 2018

I've looked at the test suite, results and everything seems to be fine on all platforms minus windows. I think something is weird with this slave.

I've logged into the windows slaves and saw the following:

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x8 pc=0x1431052]


goroutine 1 [running]:
math/big.(*Int).Int64(...)
	C:/Users/jenkins.BEATS-CI-WINDOW/.gvm/versions/go1.10.3.windows.amd64/src/math/big/int.go:365

github.com/elastic/beats/libbeat/cmd/instance.initRand()
	C:/Users/jenkins/workspace/elastic+beats+pull-request+multijob-windows/beat/filebeat/label/windows/src/github.com/elastic/beats/libbeat/cmd/instance/beat.go:153
 +0xc2
github.com/elastic/beats/libbeat/cmd/instance.init.0()
C:/Users/jenkins/workspace/elastic+beats+pull-request+multijob-windows/beat/filebeat/label/windows/src/github.com/elastic/beats/libbeat/cmd/instance/beat.go:140 +0x3b```

 Looking at the related code, this is related to when we setup the random number.

@ph
Copy link
Contributor Author

ph commented Oct 29, 2018

jenkins test this please

@ph
The 'export config' subcommand should display field reference instead of
5d75a65 
values

Change the behavior of the export config to not display the values from
the keystore or the environment.
@ph ph force-pushed the fix/export-config branch from a7f33d6 to 5d75a65 Compare October 30, 2018 11:55
@ph ph merged commit 47c78ee into elastic:master Oct 30, 2018
@ph ph mentioned this pull request Oct 30, 2018
Merged
@ph ph added v6.6.0 and removed needs_backport PR is waiting to be backported to other branches. labels Oct 30, 2018
@ph ph mentioned this pull request Oct 30, 2018
Merged
@ph ph added the v6.5.0 label Oct 30, 2018
ph added a commit to ph/beats that referenced this pull request Oct 30, 2018
@ph
The 'export config' subcommand should display field reference instead…
8263503 
… of values (elastic#8769)

Change the behavior of the export config to not display the values from
the keystore or the environment.

(cherry picked from commit 47c78ee)
@ph ph mentioned this pull request Oct 30, 2018
Closed
@ph ph added the v6.4.3 label Oct 30, 2018
ph added a commit to ph/beats that referenced this pull request Oct 30, 2018
@ph
The 'export config' subcommand should display field reference instead…
16a70fb 
… of values (elastic#8769)

Change the behavior of the export config to not display the values from
the keystore or the environment.

(cherry picked from commit 47c78ee)
@ph ph mentioned this pull request Oct 30, 2018
Merged
ph added a commit to ph/beats that referenced this pull request Oct 30, 2018
@ph
The 'export config' subcommand should display field reference instead…
9735c2b 
… of values (elastic#8769)

Change the behavior of the export config to not display the values from
the keystore or the environment.

(cherry picked from commit 47c78ee)
ph added a commit that referenced this pull request Oct 30, 2018
@ph
Cherry-pick #8769 to 6.4: The 'export config' subcommand should displ…
a8afce8 
…ay field reference instead of values (#8832)

Cherry-pick of PR #8769 to 6.4 branch. Original message:

Change the behavior of the export config to not display the values from
the keystore or the environment.
ph added a commit to ph/beats that referenced this pull request Oct 30, 2018
@ph
The 'export config' subcommand should display field reference instead…
9b5a53f 
… of values (elastic#8769)

Change the behavior of the export config to not display the values from
the keystore or the environment.

(cherry picked from commit 47c78ee)
ph added a commit to ph/beats that referenced this pull request Oct 30, 2018
@ph
The 'export config' subcommand should display field reference instead…
ec276f9 
… of values (elastic#8769)

Change the behavior of the export config to not display the values from
the keystore or the environment.

(cherry picked from commit 47c78ee)
ph added a commit that referenced this pull request Oct 31, 2018
@ph
The 'export config' subcommand should display field reference instead…
0f82c1e 
… of values (#8769) (#8817)

Change the behavior of the export config to not display the values from
the keystore or the environment.

(cherry picked from commit 47c78ee)
ph added a commit that referenced this pull request Oct 31, 2018
@ph
Cherry-pick #8769 to 6.x: The 'export config' subcommand should displ…
4d25caa 
…ay field reference instead of values (#8816)

Cherry-pick of PR #8769 to 6.x branch. Original message: 

Change the behavior of the export config to not display the values from
the keystore or the environment.
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
@ph
Cherry-pick elastic#8769 to 6.4: The 'export config' subcommand shoul…
93eed1b 
…d display field reference instead of values (elastic#8832)

Cherry-pick of PR elastic#8769 to 6.4 branch. Original message:

Change the behavior of the export config to not display the values from
the keystore or the environment.
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
@ph
The 'export config' subcommand should display field reference instead…
4976054 
… of values (elastic#8769) (elastic#8817)

Change the behavior of the export config to not display the values from
the keystore or the environment.

(cherry picked from commit 53b688b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsg tsg tsg approved these changes

@urso urso urso approved these changes

Assignees
No one assigned
Labels
libbeat review v6.4.3 v6.5.0 v6.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ph @urso @tsg