If a resource has many child tables and the user just wants a few synced, they should be able to achieve that by including those tables (and possibly all their dependent parent tables, if we don't want implicit behaviour) in the config.

This is the opposite of #340 but needs more changes in the handling side of things, not just input validation.

Seems like they both serve the same purpose of getting data from the parent.

ParentResourceFieldResolver works on table columns (probably should be called ParentColumnResolver) and goes only 1 level deep.

ParentPathResolver works on structs and can go any level deep.

Maybe we should only have a single API to access parent data? Also I think we should probably only access table columns as accessing struct data misses any post resolving done by the parent resource

Using PreResourceResolver should enable all of the resolvers to be invoked in parallel (with concurrency guardrails) this is impacting user experience that are fetching resources that require the list/detail pattern: cloudquery/cloudquery#2392

This functionality is also blocking the full removal of the ListDetail pattern in the AWS Plugin: cloudquery/cloudquery#2460

Describe the bug

With the following config:

kind: source
spec:
  name: aws
  version: v4.6.1
  destinations: [postgresql]
  path: cloudquery/aws
---
kind: source
spec:
  name: azure
  version: v1.3.3
  destinations: [postgresql]
  path: cloudquery/azure
---
kind: destination
spec:
  name: postgresql
  version: v1.6.3
  spec:
    connection_string: postgresql://postgres:pass@localhost:5432/postgres

The CLI errors out with Error: failed to load spec(s) from <path>. Error: destination postgresql is used by multiple sources cloudquery/azure with different versions

Expected Behavior

Using multiple sources should work

Steps to Reproduce

Create a YAML file with the above configuration, run cloudquery sync <file>

Possible Solution

Fix the config validation

Provider and CloudQuery version

v1.5.5

Additional Context

No response

Sometimes downloading of plugins fails due to a glitch with GitHub:
https://github.com/cloudquery/cloudquery/actions/runs/3243530449/jobs/5318376371#step:7:9

and a simple retry works.

We should add retry logic as to not fail in these cases

What's the reason we have:

kind: source
spec:
  ## Required. name of the plugin to use
  name: "aws" # required

And not:

kind: source
name: aws

Describe the bug

When I try to sync on windows using github registry cloudquery returns

Error: failed to sync source aws: failed to initialize destination plugin client for postgresql: destination configure: failed to call Configure: rpc error: code = Unavailable desc = connection error: desc = "transport: Error while dialing dial tcp: address unix://C:\\Users\\andre\\AppData\\Local\\Temp/cq-SzttZHuEzsslQeXr.sock: too many colons in address"

To achieve this result I have removed grpc.WithBlock() option in grpc.DialContext. With the option the program just blocks when cli tries to dial to plugin.

Expected Behavior

it should work on windows

Steps to Reproduce

cd cli
go run main.go sync ./examples/aws_github --log-level debug

Possible Solution

No response

Provider and CloudQuery version

main branch

Additional Context

No response

GetDestinatinoByName should be GetDestinationByName

ParentIDResolver uses an internal id (cq_id) to link between tables and relations.

Since relations are always explicitly set by a developer I think we shouldn't use an internal ID to link tables and force the developer to specify a field via ParentResourceFieldResolver.

Another reason for dropping ParentIDResolver is that our naming convention for relation id fields are <parent_resource>_id for example sql_server_id to link server<->database. (sql_server_id is on the database table).

It makes sense to populate that column with the server ID and not an internal ID

We use strcase.ToSnake to create tables and column names:

It fails in many cases which forces consumers to override columns:
https://github.com/cloudquery/cloudquery/pull/1903/files#diff-696d2ccebfefda5499a79ed759bdb24563eb1a510b7d320f8d61e37298de76deR55
https://github.com/cloudquery/cloudquery/blob/edb6e0ed01ccdb4c1b218b67d57c75c36e0815bd/plugins/source/azure/codegen/recipes/compute.go#L70

We should a library that handles those cases for us.

Maybe also do a sanity check by doing ToSnake then ToCamel and see that we get the field name

At the moment if a single write operation fails we close the gRPC destination stream and stop syncing.
Usually a write fails when a table violates primary key constraints (e.g. setting a NULL value).
I'm not sure a failure to write a single resource should prevent us from syncing all others.

Relevant code is

We pass ctx to the plugin Write method which propagates all the way down, for example:
https://github.com/cloudquery/cloudquery/blob/d74ac122a4ed1f4378a485293fa66ffee836ff29/plugins/destination/postgresql/client/write.go#L48

If there's an error during Write the context is cancelled, we fall to the case here:

(Please note we report the wrong error, which is fixed by #460)

Related to #352.

Looks like we fail to kill the plugin process on Windows:
https://github.com/cloudquery/plugin-sdk/actions/runs/3404741718/jobs/5662201307#step:6:49

We should investigate, fix, then remove //go:build !windows from clients/destination_test.go and clients/source_test.go

Describe the bug

Not sure if this belongs here or in https://github.com/cloudquery/cloudquery.

When doing go run main.go serve > output.txt in a plugin's repo (after cloning it), the file will contain the following:

�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_datalake_analytics_accounts
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_frontdoor_doors
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_datalake_store_accounts
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_compute_virtual_machines
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_eventhub_namespaces
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_cdn_profiles
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_iothub_devices
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_container_registries
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_container_managed_clusters
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_compute_disks
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_keyvault_managed_hsms
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_compute_virtual_machine_scale_sets
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_authorization_role_assignments
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_keyvault_vaults
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_batch_accounts
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_authorization_role_definitions
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_logic_workflows
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_mariadb_servers
�[90m11:40AM�[0m �[32mINF�[0m fetch start �[36mtable=�[0mazure_cosmosdb_accounts

This makes it harder to debug plugins

Expected Behavior

The output file should not have special characters

Steps to Reproduce

1. [email protected]:cloudquery/cloudquery.git
2. cd plugins/source/gcp
3. go run main.go serve > output.txt

Examine content of output.txt

Possible Solution

Support NO_COLOR=true env variable and/or automatically set it when piping output to a file

Provider and CloudQuery version

v2

Additional Context

No response

We have a couple of benchmarks now, and it would be good to run them in CI and comment on the PR if there are signifcant changes.

Looks like we can detect breaking changes to the protocol via https://docs.buf.build/ci-cd/github-actions

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Repository problems

Renovate tried to run on this repository, but found these problems.

• WARN: Package lookup failures

Awaiting Schedule

These updates are awaiting their schedule. Click on a checkbox to get an update now.

• fix(deps): Update golang.org/x/exp digest to 814bf88
• fix(deps): Update google.golang.org/genproto/googleapis/api digest to 6ceb2ff
• fix(deps): Update google.golang.org/genproto/googleapis/rpc digest to 6ceb2ff
• fix(deps): Update module github.com/klauspost/compress to v1.17.7
• fix(deps): Update module github.com/klauspost/cpuid/v2 to v2.2.7
• fix(deps): Update module github.com/tdewolff/minify/v2 to v2.20.18
• fix(deps): Update module github.com/tdewolff/parse/v2 to v2.7.12
• fix(deps): Update module github.com/ugorji/go/codec to v1.2.12
• chore(deps): Update dependency golangci/golangci-lint to v1.56.2
• fix(deps): Update module github.com/bytedance/sonic to v1.11.1
• fix(deps): Update module github.com/deepmap/oapi-codegen to v1.16.2
• fix(deps): Update module github.com/getsentry/sentry-go to v0.27.0
• fix(deps): Update module github.com/go-logr/logr to v1.4.1
• fix(deps): Update module github.com/go-playground/validator/v10 to v10.18.0
• fix(deps): Update module github.com/google/uuid to v1.6.0
• fix(deps): Update module github.com/grpc-ecosystem/grpc-gateway/v2 to v2.19.1
• fix(deps): Update module github.com/invopop/jsonschema to v0.12.0
• fix(deps): Update module github.com/leodido/go-urn to v1.4.0
• fix(deps): Update module github.com/pelletier/go-toml/v2 to v2.1.1
• fix(deps): Update module github.com/rs/zerolog to v1.32.0
• fix(deps): Update module github.com/spf13/cobra to v1.8.0
• fix(deps): Update module go.opentelemetry.io/proto/otlp to v1.1.0
• fix(deps): Update module golang.org/x/arch to v0.7.0
• fix(deps): Update module golang.org/x/crypto to v0.20.0
• fix(deps): Update module golang.org/x/mod to v0.15.0
• fix(deps): Update module golang.org/x/net to v0.21.0
• fix(deps): Update module golang.org/x/sys to v0.17.0
• fix(deps): Update module golang.org/x/tools to v0.18.0
• fix(deps): Update module google.golang.org/grpc to v1.62.0
• fix(deps): Update opentelemetry-go monorepo to v1.24.0 (go.opentelemetry.io/otel, go.opentelemetry.io/otel/exporters/otlp/otlptrace, go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp, go.opentelemetry.io/otel/metric, go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace)
• fix(deps): Update actions/checkout action to v4
• fix(deps): Update actions/github-script action to v7
• fix(deps): Update actions/setup-go action to v5
• fix(deps): Update actions/upload-artifact action to v4
• fix(deps): Update golangci/golangci-lint-action action to v4
• fix(deps): Update google-github-actions/release-please-action action to v4
• fix(deps): Update module github.com/deepmap/oapi-codegen to v2
• fix(deps): Update module github.com/flosch/pongo2/v4 to v6

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• fix(deps): Update dependency go to v1.21.6

Ignored or Blocked

These are blocked by an existing closed PR and will not be recreated unless you click a checkbox below.

• chore(deps): Update module github.com/cloudquery/plugin-sdk/v2 to v4
• fix(deps): Update module gopkg.in/yaml.v2 to v3

Detected dependencies

Currently we have Read API for destination plugins. This is mostly used by our testing framework.

One current issue is that in our testing we assume the order of read is the order of insertion but destination plugins don't have such requirements. We might need to change this API to read a specific ID rather then read everything by _cq_source_name column.

Other things to consider is potential future implementation of cursor where this might be needed.

With the new template based codegen we generate PathResolver for all columns using field names, so extracting fields always works.

Also we still have the fallback of using ToPascal on the column name to try and extract values:

We've seen casing conversions can be hard to get quite right, and we can only catch such mistakes if we have unit/mock tests for all plugins and all tables.

Should we make column.Resolver mandatory? That way plugin authors must configure how data is extracted and we don't rely on a convention that only works some of the time

introduce one flag telemetry=none,error-reporting,all

Describe the bug

Right now we implement batching in all plugins and this cause number of issues:

1. batching is mostly implemented on a X resource basis and not per table. This will mean that if there is an issue in one table it can affect all others instead of affecting just a single one.

The solution to that will be implementing batching on the SDK level for destinations.

This issue provides visibility into Renovate updates and their statuses. Learn more

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• chore(deps): Update tubone24/update_release digest to a82eaa1
• chore(deps): Update go version and linter (go, golangci/golangci-lint)
• chore(deps): Update leonsteinhaeuser/project-beta-automations action to v2
• Click on this checkbox to rebase all open PRs at once

Detected dependencies

Related to cloudquery/cloudquery#4158

I think this is low priority for now, but I'd expect the SDK to prevent deadlocks coming from plugins' code.

When generating markdown docs, we often get extra newlines that are hard to fix with Go templates themselves (while still handling all possible cases). The semantics of {{- and -}} seem too limited for this purpose.

I'm open to anyone trying to fix all the cases of unnecessary newlines using templating, but another idea is to rather run a post-processor (maybe a markdown formatter?) on the files to make them uniform. It might be as simple as replacing 3+ occurrences of \nwith \n\n.

We support env variables resolution in our config, for example:

kind: destination
spec:
  name: postgresql
  version: v1.2.0
  spec:
    connection_string: ${CQ_DSN}

However if CQ_DSN is not configured we fail with a confusing error:

Loading spec(s) from examples
Error: failed to sync source azure: failed to initialize destination plugin client for postgresql: destination configure: failed to call Configure: rpc error: code = Unknown desc = failed to connect to postgresql: failed to connect to `host=/private/tmp user=<user> database=`: dial error (dial unix /private/tmp/.s.PGSQL.5432: connect: no such file or directory)
exit status 1

We should provide a clear error message in these cases

Currently we expect all resources to resolve in less than 1 minute

ctx, cancel := context.WithTimeout(ctx, time.Minute)

I don't think that this is a large enough time value as we have already encountered 2 users that have resources that take longer than 1 minute to fully resolve.

I suggest that either we make this value much larger (like 10-20 minutes) and/or make it a configurable via the config file

Describe the bug

The following config:

kind: source
spec:
  name: aws
  version: v3.8.0
  destinations: [test,csv]
  path: cloudquery/aws
---
kind: destination
spec:
  name: test
  version: v1.2.11
  spec:
    connection_string: postgresql://postgres:pass@localhost:5432/postgres
---
kind: destination
spec:
  name: csv
  version: v1.0.0
  write_mode: append

Fails on Windows with the following error Error: expecting at least one destination

Expected Behavior

The above config should work on Windows

Steps to Reproduce

Run a sync on a Windows machine with the supplied configuration

Possible Solution

Probably related to how we split strings in

Provider and CloudQuery version

v1.5.5

Additional Context

No response

listAndValidateTables can in some circumstances return child tables right now, and this will probably cause a panic. We should either disallow child tables from being listed in the config, or filter the child tables out before returning the list if their parents are included (or both)

See comment here for details: #339 (comment)_

Background

To support overwrite-delete-stale we've added a constraint for plugins names to be unique.
Also as a result we're planning to make the path required, as to allow users to change the name.

Problem

I believe it's quite hard to communicate the reason name has to be unique, and easy for users to miss that requirement (also if users split their config across multiple machines we can't really validate it either).

Additionally since we're planning to make path required due to the uniqueness of name, it adds friction for people onboarding CloudQuery.

Proposal

Instead of making name unique we can add a dedicated id configuration (it can default to name) and use that instead.
As a result we can achieve the following:

1. Keep the meaning of name and path the same
2. Keep path optional, as we can resolve it from name
3. Users that don't need to run CloudQuery in parallel don't need to know anything about uniqueness
4. Easier to understand why an id property needs to be unique

TLDR: Only users that split their configuration (e.g. run in parallel) should care about having a unique identifier.

Flattened lists of tables have caused a few bugs now, because once they are returned it's hard to know what kind of list you are working with. Is it a flattened list or a normal list? Calling code that expects one but not the other can cause issues like tables getting traversed twice.

listAndValidateTables is one example of a function that returns a flattened list, and has so far been the main culprit

Flattened lists of tables are useful, but we should clearly distinguish flattened lists of tables from normal top-level lists. My reading is that we currently only use flattened lists for two things:

• getting the total number of tables
• checking if a table is in the top-level list by using GetTopLevel

These two things can both be achieved by a "set" type. If listAndValidateTables returned an abstracted "set" type that hid its internals and exposed only two methods: Size() int and Contains(table) bool, most of the bugs we've had so far would have been avoided. I think we should try to refactor the code to work something along those lines, so that it is easier to reason about and harder to make mistakes.

In order to help with people building orchestration on top of CloudQuery, it would be nice to be able to dump all supported tables in a machine readable way as the tables readme is a bit hard to parse using code (or copy paste even).

We could add a new command or add a --json flag to the docs command to output the result as a JSON file

Make path argument required for plugin configuration. Stop generating default path from name field

type A struct {
    Abc string
}

If we feed struct A to codegen it will naturally produce column named abc.
If we add an extra column with the same name and forget to add filed Abc to be skipped, we end up with duplicate names in the resulting resource table.

Now, this will result in cloudquery CLI failing on the sync/init part as it checks for the data integrity then. There will be something like panic: found duplicate columns in source plugin: test: duplicate column abc in table test_resource.

The proposal is to add this check to codegen, too, so that such issues are highlighted during the development.

TDLR: run the following line in a new Go routine:

so relations are resolved in parallel instead of synchronously.

We should limit the number of Go routines based on the Concurrency configuration

Describe the bug

When there are changes in table names, new doc files are added, but old files are not removed, which makes the GitHub action that checks docs fail.

Expected Behavior

remove all the files from destination directory before generating files

Steps to Reproduce

create table
generate docs
remove table
generate docs
run github action

Possible Solution

No response

Provider and CloudQuery version

Additional Context

No response

Looks like #208 moved the code that downloads plugins from the CLI to the SDK.

I'm not sure the SDK should be aware to how we structure our releases in the monorepo. We basically have a circular dependency now.

Also since we download plugins in the SDK, we lost the progress bar during download.
Also since our releases are a bit different on multiple OSes, we'll need to add cross platform tests for the SDK.

Relevant code in

We count the total number of resources including relations:

However when we print it we use the top level table name:

Which can be confusing as if the user takes the name of the table from the logs and run a count on the database, they might experience a different number of resources.

Should we only consider top level resources in the count? Maybe find a better way to print the count?

With #181, we use custom code to validate our configuration files.

We should consider using a JSON schema file and use a library to manage validation for us.

This should allow us to define the configuration in a declarative way.

Having a JSON schema should also allow code editors to auto complete configuration files based on the schema (though we shouldn't really think about this now)

The check should check if table names and columns don't have capital letters, spaces etc.

Should be useful for debugging user issues

I think we can just expose Name with the following use cases:

# GitHub `cloudquery/azure`
name: azure

# GitHub `org/cq-plugin-datadog`
name: org/cq-plugin-datadog

# Local (notice value starts with `.`)
name: ./path-to-binary

# gRPC (notice value starts with http://)
name: http://localhost:7777

We should also consider dropping Local or gRPC as they serve the same purpose.

Internally we have still parse name into Path and Registry but it doesn't need to be a part of the configuration file

Describe the bug

Some fields of AWS resources like aws_route53_traffic_policy_versions have string fields that are actually JSONs, fields like policy documents.
The check here: https://github.com/cloudquery/plugin-sdk/blob/main/plugins/source_testing.go#L98-L101
does not allow to use strings.

Expected Behavior

allow string fields to be cased as json.

Steps to Reproduce

change string column type to json

Possible Solution

remove the check

Provider and CloudQuery version

9cd9dab30204baf96ea7e78ba130258cd190eb88

Additional Context

No response

Currently we don't have a real typing system in CQ which is a pre-requirement to multiple destinations. There is wip in #298

Alot was discussed offline so sorry for the lack of details here in the issue.

TLDR: We should consider having application level errors as a part of the gRPC responses, to make it easier to identify them.

For example in the following line:

Not sure what's the best solution (need to do some research), but we could add an error(s) field to gRPC responses so we have a clear separation between application level errors and gRPC/communication level ones.

Also due to grpc/grpc-go#2934 we can't unwrap errors.

Describe the bug

If the parent has a PreResourceResolver defined, relations are called with the Resolver's output and not the "new item" from the PreResourceResolver.

This also works fine in mock tests as it should be, which is interesting.

Expected Behavior

Relation Resolver methods should be called with the result from the parent PreResourceResolver item as the parent.

Steps to Reproduce

Sentry issue SOURCE-AWS-A:

interface conversion: interface {} is types.KeyListEntry, not *types.KeyMetadata
goroutine 9995245 [running]:
runtime/debug.Stack()
	/opt/hostedtoolcache/go/1.19.2/x64/src/runtime/debug/stack.go:24 +0x65
github.com/cloudquery/plugin-sdk/plugins.(*SourcePlugin).resolveTableDfs.func1.1()
	/home/runner/go/pkg/mod/github.com/cloudquery/[email protected]/plugins/source_scheduler_dfs.go:77 +0x78
panic({0x776c700, 0xc07debab40})
	/opt/hostedtoolcache/go/1.19.2/x64/src/runtime/panic.go:884 +0x212
github.com/cloudquery/cloudquery/plugins/source/aws/resources/services/kms.fetchKmsKeyGrants({0x8a92270?, 0xc0008927b0?}, {0x89b9340?, 0xc06e902a80?}, 0x89b9340?, 0xc00030f600?)
	/home/runner/work/cloudquery/cloudquery/plugins/source/aws/resources/services/kms/key_grants_fetch.go:14 +0x3b8
github.com/cloudquery/plugin-sdk/plugins.(*SourcePlugin).resolveTableDfs.func1()
	/home/runner/go/pkg/mod/github.com/cloudquery/[email protected]/plugins/source_scheduler_dfs.go:87 +0xbb
created by github.com/cloudquery/plugin-sdk/plugins.(*SourcePlugin).resolveTableDfs
	/home/runner/go/pkg/mod/github.com/cloudquery/[email protected]/plugins/source_scheduler_dfs.go:74 +0x585

Fixing the key_grants_fetch.go to be:

	k := parent.Item.(*types.KeyListEntry)
	config := kms.ListGrantsInput{
		KeyId: k.KeyArn,

Should fix the problem temporarily, but it breaks the mock test in keys_mock_test.go. So the mock tests case works OK somehow.

Possible Solution

No response

Provider and CloudQuery version

4.14.0

Additional Context

No response

Describe the bug

When generating docs from source plugin it is expected to see all resources (even highly-nested ones) in the index.
However, only the top-level resources + 1st-level relations are shown.

Expected Behavior

All nested resources are present in the index.

Steps to Reproduce

Generate doc from provider that has relation underneath a relation.

Possible Solution

No response

Provider and CloudQuery version

current

Additional Context

No response

It has come up a couple of times that it would be good for the SDK to support cursor-based syncing. For APIs that allow it, this would allow resources to be fetched once, with only updates being fetched after the first sync. For some resource types, this optimization would be a significant performance boost.

This would be especially useful in cases where the APIs allow for time-based querying, and it is known that resources never change once they are created (like logs, reports, audit trails).

One concrete example where this would be beneficial is the Heroku audit trail event: https://devcenter.heroku.com/articles/platform-api-reference#audit-trail-event . It allows querying by day, so an optimized implementation would only fetch historical days the first time, then in subsequent runs fetch the days that have been added since the previous sync.

There is an error with this repository's Renovate configuration that needs to be fixed. As a precaution, Renovate will stop PRs until it is resolved.

Location: *.yml
Error type: Invalid regular expression: *.yml

Currently Caser using globals and not providing any way of adding options and expanding the api.

This is a heavily used public API that we need change asap to avoid breakage across all official and soon community plugins (I think yandex already uses those.)

Should follow the standard

New(required_options, ...CaserOption) pattern

I think we should discuss how we want to do dep update. having a PR for every dep is quite heavy imo and create burden (at least emotionally for me :) ).

I think if we have one PR with all deps (I think we have in our main repo) can be a better approach and maybe run the dep checks once a month should be fine or once a week, and we can go through this and decide what we want to update and what not.