Skip to content
This repository has been archived by the owner on Aug 16, 2022. It is now read-only.

fix(docs): Update documentation about adding new resources #1136

Merged
merged 2 commits into from
Jul 3, 2022

Conversation

hermanschaaf
Copy link
Member

Some small fixes and updates to adding_a_new_resource.md for correctness.

@hermanschaaf hermanschaaf requested a review from a team as a code owner July 1, 2022 14:46
@hermanschaaf hermanschaaf requested review from yevgenypats and removed request for a team July 1, 2022 14:46
Comment on lines 64 to 65
export AWS_ACCESS_KEY_ID={Your AWS Access Key ID}
export AWS_SECRET_ACCESS_KEY={Your AWS secret access key}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say it is better to set the AWS_PROFILE environment variable. That way credentials can be managed in the ~/.aws/config and can use any of credential sources like AWS_SSO, credential_process or worst case scenario hard coded credentials

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough! I think AWS_SECRET_ACCESS_KEY is probably the easiest way to get started quickly, so it's worth mentioning we support that too. I've changed it now to recommend AWS_PROFILE, but mention that we support other methods in a comment. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that we cannot not support hard coded credentials! I think that we should be helping/pushing users to follow the best practices. I like that idea of putting the link to the docs! Here is a good article about this

Copy link
Member

Choose a reason for hiding this comment

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

We can just link to this one - https://hub.cloudquery.io/providers/cloudquery/aws/latest#authentication
This will also serve as nice cross link to our docs

@hermanschaaf
Copy link
Member Author

Looks like the unit tests are failing for some unrelated reason:

--- FAIL: TestCloudwatchlogsFilter (0.05s)
    filters_mock_test.go:31: fetch resources [test_resource]
    provider.go:257: [INFO] calculated max goroutines for fetch execution [max_goroutines 65536]
    execution.go:251: [INFO] fetched successfully [count 1]
    --- FAIL: TestCloudwatchlogsFilter/aws_cloudwatchlogs_filters (0.00s)
        --- FAIL: TestCloudwatchlogsFilter/aws_cloudwatchlogs_filters/aws_cloudwatchlogs_filter_metric_transformations (0.00s)
            filters_mock_test.go:31: expected to have at least 1 entry at table aws_cloudwatchlogs_filter_metric_transformations got zero
FAIL
FAIL	github.com/cloudquery/cq-provider-aws/resources/services/cloudwatchlogs	0.072s

@yevgenypats or @bbernays do you happen to know something about this? Otherwise I can investigate

@bbernays
Copy link
Contributor

bbernays commented Jul 1, 2022

Looks like the unit tests are failing for some unrelated reason:

--- FAIL: TestCloudwatchlogsFilter (0.05s)
    filters_mock_test.go:31: fetch resources [test_resource]
    provider.go:257: [INFO] calculated max goroutines for fetch execution [max_goroutines 65536]
    execution.go:251: [INFO] fetched successfully [count 1]
    --- FAIL: TestCloudwatchlogsFilter/aws_cloudwatchlogs_filters (0.00s)
        --- FAIL: TestCloudwatchlogsFilter/aws_cloudwatchlogs_filters/aws_cloudwatchlogs_filter_metric_transformations (0.00s)
            filters_mock_test.go:31: expected to have at least 1 entry at table aws_cloudwatchlogs_filter_metric_transformations got zero
FAIL
FAIL	github.com/cloudquery/cq-provider-aws/resources/services/cloudwatchlogs	0.072s

@yevgenypats or @bbernays do you happen to know something about this? Otherwise I can investigate

I have never seen this occur before. Maybe it is just the go-faker that randomly set a value to nil (which is a valid value) and in this test we were testing for inserting data into all nested tables?

If we re-run the check does it pass?

@yevgenypats yevgenypats merged commit fe5a5ad into cloudquery:main Jul 3, 2022
kodiakhq bot pushed a commit that referenced this pull request Jul 4, 2022
🤖 I have created a release *beep* *boop*
---


## [0.12.25](v0.12.24...v0.12.25) (2022-07-04)


### Bug Fixes

* **deps:** Update github.com/gocarina/gocsv digest to 72f2e84 ([#1151](#1151)) ([1c870e8](1c870e8))
* **deps:** Update module github.com/aws/aws-sdk-go-v2 to v1.16.6 ([#1152](#1152)) ([4ac2583](4ac2583))
* **deps:** Update module github.com/aws/aws-sdk-go-v2/service/accessanalyzer to v1.15.7 ([#1153](#1153)) ([9ed2343](9ed2343))
* **deps:** Update module github.com/aws/aws-sdk-go-v2/service/codepipeline to v1.13.7 ([#1154](#1154)) ([0a34f8e](0a34f8e))
* **deps:** Update module github.com/cloudquery/cq-provider-sdk to v0.13.3 ([#1156](#1156)) ([d4b0701](d4b0701))
* **docs:** Update documentation about adding new resources ([#1136](#1136)) ([fe5a5ad](fe5a5ad))
* **tests:** Update Lightsail terraform for Integration testing of Instances ([#1137](#1137)) ([488a003](488a003))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
erezrokah pushed a commit to cloudquery/cloudquery that referenced this pull request Aug 14, 2022
🤖 I have created a release *beep* *boop*
---


## [0.12.25](cloudquery/cq-provider-aws@v0.12.24...v0.12.25) (2022-07-04)


### Bug Fixes

* **deps:** Update github.com/gocarina/gocsv digest to 72f2e84 ([#1151](cloudquery/cq-provider-aws#1151)) ([1c870e8](cloudquery/cq-provider-aws@1c870e8))
* **deps:** Update module github.com/aws/aws-sdk-go-v2 to v1.16.6 ([#1152](cloudquery/cq-provider-aws#1152)) ([4ac2583](cloudquery/cq-provider-aws@4ac2583))
* **deps:** Update module github.com/aws/aws-sdk-go-v2/service/accessanalyzer to v1.15.7 ([#1153](cloudquery/cq-provider-aws#1153)) ([9ed2343](cloudquery/cq-provider-aws@9ed2343))
* **deps:** Update module github.com/aws/aws-sdk-go-v2/service/codepipeline to v1.13.7 ([#1154](cloudquery/cq-provider-aws#1154)) ([0a34f8e](cloudquery/cq-provider-aws@0a34f8e))
* **deps:** Update module github.com/cloudquery/cq-provider-sdk to v0.13.3 ([#1156](cloudquery/cq-provider-aws#1156)) ([d4b0701](cloudquery/cq-provider-aws@d4b0701))
* **docs:** Update documentation about adding new resources ([#1136](cloudquery/cq-provider-aws#1136)) ([fe5a5ad](cloudquery/cq-provider-aws@fe5a5ad))
* **tests:** Update Lightsail terraform for Integration testing of Instances ([#1137](cloudquery/cq-provider-aws#1137)) ([488a003](cloudquery/cq-provider-aws@488a003))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants