Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Semver not mandatory
  • Loading branch information
jsoriano committed Jan 13, 2025
commit b2e2f5b3aaea76cebebf96cdaf73a90c6c1f0b91
4 changes: 2 additions & 2 deletions internal/kibana/agents.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (c *Client) QueryAgents(ctx context.Context, kuery string) ([]Agent, error)
}

switch {
case c.semver.Major() < 9:
case c.semver != nil && c.semver.Major() < 9:
return resp.List, nil
default:
return resp.Items, nil
Expand All @@ -96,7 +96,7 @@ func (c *Client) AssignPolicyToAgent(ctx context.Context, a Agent, p Policy) err
var err error
var respBody []byte
switch {
case c.semver.Major() < 9:
case c.semver != nil && c.semver.Major() < 9:
statusCode, respBody, err = c.put(ctx, path, []byte(reqBody))
default:
statusCode, respBody, err = c.post(ctx, path, []byte(reqBody))
Expand Down
6 changes: 2 additions & 4 deletions internal/kibana/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,8 @@ func NewClient(opts ...ClientOption) (*Client, error) {
}
c.versionInfo = v.Version

if c.versionInfo.Number == "" {
// Version info may not contain any version if this is a managed Kibana.
c.semver = semver.MustParse("9.0.0")
} else {
// Version info may not contain any version if this is a managed Kibana.
if c.versionInfo.Number != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this happen in Serverless projects ? Should we set a default version in that case or just not set it at all ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this happens in serverless.

Not sure of what would be better.

In my other branch I was setting 9.0.0 (here). But it can be tricky to chose a proper version, and remember to update it if it is not something like 99.99.99.

So here I am opting by no setting it at all, what indicates that it is a deployment with an unknown version. The only known case for this is serverless, so it is in principle safe to assume that is the latest version. The problem with this approach is that we have to remember to do null checks.

An alternative I considered was to create an interface with the features that are dependant of the version, and set the proper implementation depending on the info here.

Copy link
Contributor

Choose a reason for hiding this comment

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

So here I am opting by no setting it at all, what indicates that it is a deployment with an unknown version. The only known case for this is serverless, so it is in principle safe to assume that is the latest version.

It looks reasonable to assume that, ok 👍

c.semver, err = semver.NewVersion(c.versionInfo.Number)
if err != nil {
return nil, fmt.Errorf("failed to parse Kibana version (%s): %w", c.versionInfo.Number, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/kibana/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func (c *Client) epmPackageUrl(name, version string) string {
return fmt.Sprintf("%s/epm/packages/%s", FleetAPI, name)
}
switch {
case c.semver.Major() < 8:
case c.semver != nil && c.semver.Major() < 8:
return fmt.Sprintf("%s/epm/packages/%s-%s", FleetAPI, name, version)
default:
return fmt.Sprintf("%s/epm/packages/%s/%s", FleetAPI, name, version)
Expand Down