Skip to content

Commit

Permalink
chore: Small fixes to logging / page size (#265)
Browse files Browse the repository at this point in the history
  • Loading branch information
traut authored Dec 7, 2024
1 parent cb3ceb4 commit 8c040aa
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 26 deletions.
4 changes: 3 additions & 1 deletion engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ func (e *Engine) LoadPluginResolver(ctx context.Context, includeRemote bool) (di
ctx, span := e.tracer.Start(ctx, "Engine.LoadPluginResolver", trace.WithAttributes(
attribute.String("includeRemote", fmt.Sprint(includeRemote)),
))
e.logger.DebugContext(ctx, "Loading plugin resolver", "includeRemote", includeRemote)
defer func() {
if diags.HasErrors() {
span.RecordError(diags)
Expand All @@ -185,6 +184,9 @@ func (e *Engine) LoadPluginResolver(ctx context.Context, includeRemote bool) (di
sources := []resolver.Source{
resolver.NewLocal(pluginDir, e.logger, e.tracer),
}

e.logger.DebugContext(ctx, "Loading plugin resolver", "include_remote", includeRemote, "plugins_dir", string(pluginDir))

if e.config.PluginRegistry != nil {
if e.config.PluginRegistry.MirrorDir != "" {
mirrorDirInfo, err := os.Stat(e.config.PluginRegistry.MirrorDir)
Expand Down
49 changes: 35 additions & 14 deletions internal/microsoft/client/graph_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (

const (
baseURLGraph = "https://graph.microsoft.com"
defaultPageSizeGraph = 200
defaultPageSizeGraph = 50
)

type graphClient struct {
Expand Down Expand Up @@ -57,6 +57,7 @@ func (client *graphClient) fetchURL(ctx context.Context, requestUrl *url.URL) (r
err = fmt.Errorf("Microsoft Graph client returned status code: %d", res.StatusCode)
return
}

result, err = plugindata.UnmarshalJSON(raw)
if err != nil {
return nil, fmt.Errorf("failed to unmarshal results: %s", err)
Expand All @@ -82,15 +83,22 @@ func (client *graphClient) QueryObjects(
queryParams = url.Values{}
}

limit := min(size, defaultPageSizeGraph)
queryParams.Set("$top", strconv.Itoa(limit))

// limit := min(size, defaultPageSizeGraph)
// $top doesn't work for managedDevices
// queryParams.Set("$top", strconv.Itoa(limit))
// queryParams.Set("$count", "true")
requestUrl.RawQuery = queryParams.Encode()

var totalCount int = -1
var response plugindata.Data

for {

if totalCount > 0 {
queryParams.Set("$skip", strconv.Itoa(len(objects)))
requestUrl.RawQuery = queryParams.Encode()
}

slog.DebugContext(ctx, "Fetching a page from Microsoft Graph API", "url", requestUrl.String())
response, err = client.fetchURL(ctx, requestUrl)
if err != nil {
Expand Down Expand Up @@ -137,16 +145,29 @@ func (client *graphClient) QueryObjects(

nextLink, ok := resultMap["@odata.nextLink"]
if !ok && nextLink == nil {
break
}
requestUrlRaw, ok := nextLink.(plugindata.String)
if !ok {
return nil, fmt.Errorf("unexpected value type for `@odata.nextLink`: %T", requestUrlRaw)
}
requestUrl, err = url.Parse(string(requestUrlRaw))
if err != nil {
slog.DebugContext(ctx, "Can't parse the next link in Microsoft Graph API response", "value", requestUrlRaw)
return nil, err
slog.DebugContext(ctx, "No `@odata.nextLink` found in the response")

if totalCount < 0 {
slog.DebugContext(ctx, "Total count is not known, breaking")
break
}

// Check totalCount only if there is no nextLink -- sometimes the response has
// the count set to the $top value and nextLink is present
if totalCount > 0 && len(objects) >= totalCount {
break
}

} else {
requestUrlRaw, ok := nextLink.(plugindata.String)
if !ok {
return nil, fmt.Errorf("unexpected value type for `@odata.nextLink`: %T", requestUrlRaw)
}
requestUrl, err = url.Parse(string(requestUrlRaw))
if err != nil {
slog.DebugContext(ctx, "Can't parse the next link in Microsoft Graph API response", "value", requestUrlRaw)
return nil, err
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion plugin/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func NewResolver(constraints map[string]string, opts ...Option) (*Resolver, diag
// Install all plugins based the version constraints and return updated a lock file.
func (r *Resolver) Install(ctx context.Context, lockFile *LockFile, upgrade bool) (_ *LockFile, diags diagnostics.Diag) {
ctx, span := r.tracer.Start(ctx, "Resolver.Install")
r.logger.InfoContext(ctx, "Installing plugins", "upgrade", upgrade)
r.logger.InfoContext(ctx, "Resolving and installing plugin dependencies", "upgrade", upgrade)
defer func() {
if diags.HasErrors() {
span.RecordError(diags)
Expand Down
57 changes: 47 additions & 10 deletions plugin/resolver/source_remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,12 @@ func (source RemoteSource) fetchVersions(ctx context.Context, name Name) (_ []re
}

// Resolve returns the binary path and checksum for the given plugin version.
func (source RemoteSource) Resolve(ctx context.Context, name Name, version Version, checksums []Checksum) (_ *ResolvedPlugin, err error) {
func (source RemoteSource) Resolve(
ctx context.Context,
name Name,
version Version,
checksums []Checksum,
) (_ *ResolvedPlugin, err error) {
ctx, span := source.tracer.Start(ctx, "RemoteSource.Resolve",
trace.WithAttributes(
attribute.String("name", name.String()),
Expand All @@ -233,7 +238,11 @@ func (source RemoteSource) Resolve(ctx context.Context, name Name, version Versi
}

// fetchDownloadInfo resolves the download info for sthe given plugin version from the registry.
func (source RemoteSource) fetchDownloadInfo(ctx context.Context, name Name, version Version) (_ *regDownloadInfo, err error) {
func (source RemoteSource) fetchDownloadInfo(
ctx context.Context,
name Name,
version Version,
) (_ *regDownloadInfo, err error) {
ctx, span := source.tracer.Start(ctx, "RemoteSource.fetchDownloadInfo",
trace.WithAttributes(
attribute.String("name", name.String()),
Expand Down Expand Up @@ -289,7 +298,13 @@ func (source RemoteSource) fetchChecksums(ctx context.Context, name Name, versio
}
span.End()
}()
url := fmt.Sprintf("%s/v1/plugins/%s/%s/%s/checksums", source.baseURL, name.Namespace(), name.Short(), version.String())
url := fmt.Sprintf(
"%s/v1/plugins/%s/%s/%s/checksums",
source.baseURL,
name.Namespace(),
name.Short(),
version.String(),
)
req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
if err != nil {
return nil, err
Expand All @@ -310,14 +325,20 @@ func (source RemoteSource) fetchChecksums(ctx context.Context, name Name, versio
}

// download downloads the plugin from the registry and returns the binary path and checksum.
func (source RemoteSource) download(ctx context.Context, name Name, version Version, info *regDownloadInfo, checksums []Checksum) (_ *ResolvedPlugin, err error) {
func (source RemoteSource) download(
ctx context.Context,
name Name,
version Version,
info *regDownloadInfo,
checksums []Checksum,
) (_ *ResolvedPlugin, err error) {
ctx, span := source.tracer.Start(ctx, "RemoteSource.download",
trace.WithAttributes(
attribute.String("name", name.String()),
attribute.String("version", version.String()),
),
)
source.logger.InfoContext(ctx, "Downloading plugin", "name", name, "version", version)
source.logger.InfoContext(ctx, "Downloading a plugin", "name", name, "version", version)
defer func() {
if err != nil {
span.RecordError(err)
Expand All @@ -330,18 +351,18 @@ func (source RemoteSource) download(ctx context.Context, name Name, version Vers
var err error
checksums, err = source.fetchChecksums(ctx, name, version)
if err != nil {
return nil, fmt.Errorf("failed to fetch plugin checksums: %w", err)
return nil, fmt.Errorf("failed to fetch the plugin checksums: %w", err)
}
}
// make a http request to download the plugin
req, err := http.NewRequestWithContext(ctx, "GET", info.DownloadURL, nil)
if err != nil {
return nil, fmt.Errorf("failed to create download request: %w", err)
return nil, fmt.Errorf("failed to create a download request: %w", err)
}
req.Header.Set("Accept", "application/octet-stream")
resp, err := source.call(req, downloadTimeout)
if err != nil {
return nil, fmt.Errorf("failed to download plugin: %w", err)
return nil, fmt.Errorf("failed to download a plugin: %w", err)
}
defer resp.Body.Close()
// verify download response headers
Expand All @@ -356,6 +377,13 @@ func (source RemoteSource) download(ctx context.Context, name Name, version Vers
if err != nil {
return nil, fmt.Errorf("failed to extract plugin: %w", err)
}
source.logger.DebugContext(
ctx,
"Plugin downloaded, verified and stored in a local cache",
"name", name,
"version", version,
"path", string(binaryPath),
)
// cleanup extracted files if there is an error during checksum verification
defer func() {
if err == nil {
Expand Down Expand Up @@ -394,7 +422,11 @@ func (source RemoteSource) download(ctx context.Context, name Name, version Vers
func (source RemoteSource) verifyDownloadHeaders(res *http.Response) error {
// verify the download size
if res.ContentLength > maxDownloadSize {
return fmt.Errorf("plugin download size exceeds the limit, got = %d, expect < %d", res.ContentLength, maxDownloadSize)
return fmt.Errorf(
"plugin download size exceeds the limit, got = %d, expect < %d",
res.ContentLength,
maxDownloadSize,
)
}
disposition, params, err := mime.ParseMediaType(res.Header.Get("Content-Disposition"))
if err != nil {
Expand All @@ -414,7 +446,12 @@ func (source RemoteSource) verifyDownloadHeaders(res *http.Response) error {
}

// extract the plugin from the tar.gz file and returns the binary and checksum file path.
func (source RemoteSource) extract(name Name, version Version, archive io.Reader, checksums []Checksum) (binPath, sumPath string, err error) {
func (source RemoteSource) extract(
name Name,
version Version,
archive io.Reader,
checksums []Checksum,
) (binPath, sumPath string, err error) {
read, err := gzip.NewReader(archive)
if err != nil {
return "", "", fmt.Errorf("failed to create gzip reader: %w", err)
Expand Down

0 comments on commit 8c040aa

Please sign in to comment.