Skip to content

chore: Fix linter findings for revive:exported in plugins/secretstores #17017

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

Merged
merged 2 commits into from
May 28, 2025

Conversation

zak-pawel
Copy link
Collaborator

Summary

Address findings for revive:exported in plugins/secretstores.

As part of this effort for files from plugins/secretstores, the following actions were taken:

  • Type names (const, var, struct, func, etc) were changed to unexported, wherever they didn't need to be exported.
  • All remaining exported types were given comments in the appropriate form – this does not apply to exported methods that implement "known" plugin interfaces (Init |SampleConfig |Gather |Start |Stop |GetState |SetState |SetParser |SetParserFunc |SetTranslator |Probe |Get |Set |List |GetResolver ).
  • The order of methods was organized (exported methods first, then unexported, with init at the very end).

It is only part of the bigger work (for issue: #15813).

Checklist

  • No AI generated code was used in this PR

Copy link
Contributor

@skartikey skartikey left a comment

Choose a reason for hiding this comment

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

@zak-pawel Thanks for the contribution!

@skartikey skartikey removed their assignment May 23, 2025
@skartikey skartikey added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label May 23, 2025
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

@zak-pawel I think we agreed on keeping configuration structs exported, didn't we?

Comment on lines -16 to +20
type AesEncryptor struct {
type aesEncryptor struct {
Variant []string `toml:"-"`
Key config.Secret `toml:"key"`
Vec config.Secret `toml:"init_vector"`
KDFConfig
kdfConfig
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm didn't we agree on exporting configuration structs?

Comment on lines -13 to 16
type DecryptionConfig struct {
type decryptionConfig struct {
Cipher string `toml:"cipher"`
Aes AesEncryptor `toml:"aes"`
Aes aesEncryptor `toml:"aes"`
}
Copy link
Member

Choose a reason for hiding this comment

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

Config struct...

Comment on lines -16 to 19
type KDFConfig struct {
type kdfConfig struct {
Algorithm string `toml:"kdf_algorithm"`
Passwd config.Secret `toml:"password"`
Salt config.Secret `toml:"salt"`
Copy link
Member

Choose a reason for hiding this comment

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

Config struct.

Comment on lines +37 to +44
type tokenConfig struct {
Key string `toml:"key"`
ClientID config.Secret `toml:"client_id"`
ClientSecret config.Secret `toml:"client_secret"`
Scopes []string `toml:"scopes"`
Params map[string]string `toml:"parameters"`
}

Copy link
Member

Choose a reason for hiding this comment

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

Config struct

@zak-pawel
Copy link
Collaborator Author

@zak-pawel I think we agreed on keeping configuration structs exported, didn't we?

@srebhan Hmmm, I don't remember the details, but I think we agreed on that for the "main" struct of each plugin.
In the input plugins, I changed dozens of config structs (not the "main" ones) to unexported, and everyone was happy 🤷‍♂️

@srebhan
Copy link
Member

srebhan commented May 28, 2025

@zak-pawel, OK let's keep them unexported then. I remember that we had some issues somewhere. IIRC this would prevent any future config helper tool to determine the type of the TOML setting which is only avail in the struct... But as I miss out on the details and everything seems to work, I'm not holding up this PR. In the worst case we need to export those again...

@srebhan srebhan merged commit ec0197f into influxdata:master May 28, 2025
23 of 24 checks passed
@github-actions github-actions bot added this to the v1.35.0 milestone May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore linter ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants