Skip to content
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

tests/provider: Fix and enable wrapcheck linter #15892

Closed
bflad opened this issue Oct 28, 2020 · 5 comments
Closed

tests/provider: Fix and enable wrapcheck linter #15892

bflad opened this issue Oct 28, 2020 · 5 comments
Labels
linter Pertains to changes to or issues with the various linters. stale Old or inactive issues managed by automation, if no further action taken these will get closed. technical-debt Addresses areas of the codebase that need refactoring or redesign. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.

Comments

@bflad
Copy link
Contributor

bflad commented Oct 28, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

golangci-lint version 1.32.0 introduced the wrapcheck linter, which by its own description:

A simple Go linter to check that errors from external packages are wrapped during return to help identify the error source during debugging.

This is great because this is a common review item where we prefer to return error message context about when/where an error is occurring. This linter is a little more focused than the goerr113 linter, which is also in golangci-lint but is problematic from our perspective due to:

Also, any call of errors.New() and fmt.Errorf() methods are reported except the calls used to initialise package-level variables and the fmt.Errorf() calls wrapping the other errors.

We do this regularly as this project is not intended to be used as a library.

Example Reports

aws/data_source_aws_ami.go:207:10: error returned from external package is unwrapped (wrapcheck)
		return err
		       ^
aws/data_source_aws_ami.go:292:10: error returned from external package is unwrapped (wrapcheck)
		return err
		       ^
aws/data_source_aws_ami.go:295:10: error returned from external package is unwrapped (wrapcheck)
		return err
		       ^
aws/data_source_aws_ami.go:298:10: error returned from external package is unwrapped (wrapcheck)
		return err
		       ^
aws/data_source_aws_ami_ids.go:73:10: error returned from external package is unwrapped (wrapcheck)
		return err
		       ^
aws/data_source_aws_api_gateway_api_key.go:61:10: error returned from external package is unwrapped (wrapcheck)
		return err
		       ^
aws/data_source_aws_api_gateway_rest_api.go:159:9: error returned from external package is unwrapped (wrapcheck)
	return err
	       ^

Affected Resources

TBD

Definition of Done

  • Reports via golangci-lint run --enable wrapcheck ./aws are addressed
  • wrapcheck added to .golangci.yml
  • CI testing passes with linter enabled
@bflad bflad added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. technical-debt Addresses areas of the codebase that need refactoring or redesign. linter Pertains to changes to or issues with the various linters. labels Oct 28, 2020
@mattburgess
Copy link
Collaborator

mattburgess commented Dec 14, 2020

I'm gonna pick this up if folks don't mind. I initially got lulled into a false sense of security by running golangci-lint run --enable wrapcheck ./aws. It turns out that running golangci-lint --max-issues-per-linter 0 run ./aws is far more illuminating as to the scale of the fixes required 😄

Here's the full list of data sources and resources affected; their acceptance tests are also affected. Some ancilliary files also show hits which will probably get fixed up in the final PR in the series which will enable the linter:

  • data_source_aws_ami
  • data_source_aws_ami_ids
  • data_source_aws_api_gateway_api_key
  • data_source_aws_api_gateway_rest_api
  • data_source_aws_availability_zone
  • data_source_aws_batch_compute_environment
  • data_source_aws_batch_job_queue
  • data_source_aws_canonical_user_id
  • data_source_aws_cloudformation_stack
  • data_source_aws_cognito_user_pools
  • data_source_aws_db_cluster_snapshot
  • data_source_aws_db_event_categories
  • data_source_aws_db_instance
  • data_source_aws_db_snapshot
  • data_source_aws_dynamodb_table
  • data_source_aws_ebs_snapshot
  • data_source_aws_ebs_snapshot_ids
  • data_source_aws_ebs_volume
  • data_source_aws_ec2_instance_type
  • data_source_aws_ecs_cluster
  • data_source_aws_ecs_container_definition
  • data_source_aws_ecs_service
  • data_source_aws_efs_mount_target
  • data_source_aws_elasticache_cluster
  • data_source_aws_elastic_beanstalk_solution_stack
  • data_source_aws_guardduty_detector
  • data_source_aws_iam_account_alias
  • data_source_aws_iam_policy_document
  • data_source_aws_iam_server_certificate
  • data_source_aws_instance
  • data_source_aws_instances
  • data_source_aws_internet_gateway
  • data_source_aws_kms_ciphertext
  • data_source_aws_lambda_invocation
  • data_source_aws_lb_listener
  • data_source_aws_nat_gateway
  • data_source_aws_network_acls
  • data_source_aws_network_interface
  • data_source_aws_network_interfaces
  • data_source_aws_prefix_list
  • data_source_aws_redshift_cluster
  • data_source_aws_region
  • data_source_aws_route53_zone
  • data_source_aws_route
  • data_source_aws_route_table
  • data_source_aws_route_tables
  • data_source_aws_s3_bucket
  • data_source_aws_security_group
  • data_source_aws_security_groups
  • data_source_aws_subnet
  • data_source_aws_subnet_ids
  • data_source_aws_vpc
  • data_source_aws_vpc_peering_connection
  • data_source_aws_vpc_peering_connections
  • data_source_aws_vpcs
  • data_source_aws_vpn_gateway
  • data_source_aws_workspaces_bundle
  • data_source_aws_workspaces_workspace
  • resource_aws_acm_certificate_validation
  • resource_aws_ami_copy
  • resource_aws_ami_from_instance
  • resource_aws_ami
  • resource_aws_ami_launch_permission
  • resource_aws_api_gateway_account
  • resource_aws_api_gateway_api_key
  • resource_aws_api_gateway_authorizer
  • resource_aws_api_gateway_base_path_mapping
  • resource_aws_api_gateway_client_certificate
  • resource_aws_api_gateway_deployment
  • resource_aws_api_gateway_documentation_part
  • resource_aws_api_gateway_documentation_version
  • resource_aws_api_gateway_domain_name
  • resource_aws_api_gateway_gateway_response
  • resource_aws_api_gateway_integration
  • resource_aws_api_gateway_integration_response
  • resource_aws_api_gateway_method
  • resource_aws_api_gateway_method_response
  • resource_aws_api_gateway_method_settings
  • resource_aws_api_gateway_model
  • resource_aws_api_gateway_request_validator
  • resource_aws_api_gateway_resource
  • resource_aws_api_gateway_rest_api
  • resource_aws_api_gateway_stage
  • resource_aws_api_gateway_usage_plan
  • resource_aws_api_gateway_usage_plan_key
  • resource_aws_apigatewayv2_domain_name
  • resource_aws_apigatewayv2_integration
  • resource_aws_apigatewayv2_route
  • resource_aws_apigatewayv2_stage
  • resource_aws_api_gateway_vpc_link
  • resource_aws_appautoscaling_scheduled_action
  • resource_aws_appmesh_gateway_route
  • resource_aws_appmesh_route
  • resource_aws_appmesh_virtual_gateway
  • resource_aws_appmesh_virtual_node
  • resource_aws_appmesh_virtual_router
  • resource_aws_appmesh_virtual_service
  • resource_aws_appsync_api_key
  • resource_aws_appsync_datasource
  • resource_aws_appsync_graphql_api
  • resource_aws_athena_database
  • resource_aws_athena_named_query
  • resource_aws_autoscaling_group
  • resource_aws_autoscaling_group_waiting
  • resource_aws_autoscaling_notification
  • resource_aws_autoscaling_policy
  • resource_aws_batch_compute_environment
  • resource_aws_batch_job_definition
  • resource_aws_batch_job_queue
  • resource_aws_budgets_budget
  • resource_aws_cloud9_environment_ec2
  • resource_aws_cloudformation_stack
  • resource_aws_cloudformation_stack_set
  • resource_aws_cloudformation_stack_set_instance
  • resource_aws_cloudfront_distribution
  • resource_aws_cloudfront_origin_access_identity
  • resource_aws_cloudfront_public_key
  • resource_aws_cloudhsm2_cluster
  • resource_aws_cloudhsm2_hsm
  • resource_aws_cloudtrail
  • resource_aws_cloudwatch_event_permission
  • resource_aws_cloudwatch_event_rule
  • resource_aws_cloudwatch_event_target
  • resource_aws_cloudwatch_log_destination
  • resource_aws_cloudwatch_log_group
  • resource_aws_cloudwatch_log_resource_policy
  • resource_aws_cloudwatch_log_stream
  • resource_aws_cloudwatch_metric_alarm
  • resource_aws_codeartifact_domain
  • resource_aws_codeartifact_repository
  • resource_aws_codebuild_project
  • resource_aws_codebuild_webhook
  • resource_aws_codecommit_trigger
  • resource_aws_codedeploy_app
  • resource_aws_codedeploy_deployment_config
  • resource_aws_codedeploy_deployment_group
  • resource_aws_codepipeline
  • resource_aws_codepipeline_webhook
  • resource_aws_codestarnotifications_notification_rule
  • resource_aws_cognito_identity_pool
  • resource_aws_cognito_identity_pool_roles_attachment
  • resource_aws_cognito_identity_provider
  • resource_aws_cognito_resource_server
  • resource_aws_cognito_user_pool_client
  • resource_aws_cognito_user_pool_domain
  • resource_aws_config_aggregate_authorization
  • resource_aws_config_config_rule
  • resource_aws_config_configuration_aggregator
  • resource_aws_config_remediation_configuration
  • resource_aws_cur_report_definition
  • resource_aws_customer_gateway
  • resource_aws_datapipeline_pipeline
  • resource_aws_dax_cluster
  • resource_aws_dax_parameter_group
  • resource_aws_dax_subnet_group
  • resource_aws_db_event_subscription
  • resource_aws_db_instance
  • resource_aws_db_instance_role_association
  • resource_aws_db_parameter_group
  • resource_aws_db_proxy_default_target_group
  • resource_aws_db_proxy
  • resource_aws_db_security_group
  • resource_aws_db_snapshot
  • resource_aws_db_subnet_group
  • resource_aws_default_network_acl
  • resource_aws_default_route_table
  • resource_aws_default_security_group
  • resource_aws_default_subnet
  • resource_aws_default_vpc
  • resource_aws_directory_service_conditional_forwarder
  • resource_aws_directory_service_directory
  • resource_aws_dms_certificate
  • resource_aws_dms_endpoint
  • resource_aws_dms_event_subscription
  • resource_aws_dms_replication_instance
  • resource_aws_dms_replication_subnet_group
  • resource_aws_dms_replication_task
  • resource_aws_docdb_cluster
  • resource_aws_docdb_cluster_parameter_group
  • resource_aws_dx_bgp_peer
  • resource_aws_dx_connection_association
  • resource_aws_dx_connection
  • resource_aws_dx_gateway_association
  • resource_aws_dx_gateway_association_migrate
  • resource_aws_dx_gateway_association_proposal
  • resource_aws_dx_gateway
  • resource_aws_dx_lag
  • resource_aws_dynamodb_global_table
  • resource_aws_dynamodb_table
  • resource_aws_dynamodb_table_item
  • resource_aws_dynamodb_table_migrate
  • resource_aws_ebs_encryption_by_default
  • resource_aws_ebs_snapshot_copy
  • resource_aws_ebs_snapshot
  • resource_aws_ebs_volume
  • resource_aws_ec2_availability_zone_group
  • resource_aws_ec2_client_vpn_authorization_rule
  • resource_aws_ec2_client_vpn_endpoint
  • resource_aws_ec2_client_vpn_network_association
  • resource_aws_ec2_client_vpn_route
  • resource_aws_ec2_local_gateway_route
  • resource_aws_ec2_local_gateway_route_table_vpc_association
  • resource_aws_ecr_lifecycle_policy
  • resource_aws_ecr_repository_policy
  • resource_aws_ecs_cluster
  • resource_aws_ecs_service
  • resource_aws_ecs_task_definition
  • resource_aws_ecs_task_definition_migrate
  • resource_aws_efs_access_point
  • resource_aws_efs_file_system
  • resource_aws_efs_mount_target
  • resource_aws_eip_association
  • resource_aws_eip
  • resource_aws_eks_cluster
  • resource_aws_eks_fargate_profile
  • resource_aws_eks_node_group
  • resource_aws_elasticache_cluster
  • resource_aws_elasticache_parameter_group
  • resource_aws_elasticache_replication_group
  • resource_aws_elasticache_security_group
  • resource_aws_elasticache_subnet_group
  • resource_aws_elastic_beanstalk_application
  • resource_aws_elastic_beanstalk_application_version
  • resource_aws_elastic_beanstalk_configuration_template
  • resource_aws_elastic_beanstalk_environment
  • resource_aws_elasticsearch_domain
  • resource_aws_elasticsearch_domain_policy
  • resource_aws_elastic_transcoder_pipeline
  • resource_aws_elastic_transcoder_preset
  • resource_aws_elb
  • resource_aws_emr_cluster
  • resource_aws_emr_instance_group
  • resource_aws_emr_security_configuration
  • resource_aws_fms_admin_account
  • resource_aws_gamelift_alias
  • resource_aws_gamelift_build
  • resource_aws_gamelift_fleet
  • resource_aws_glacier_vault
  • resource_aws_glacier_vault_lock
  • resource_aws_globalaccelerator_accelerator
  • resource_aws_globalaccelerator_listener
  • resource_aws_glue_catalog_database
  • resource_aws_glue_classifier
  • resource_aws_glue_connection
  • resource_aws_glue_job
  • resource_aws_glue_partition
  • resource_aws_glue_security_configuration
  • resource_aws_glue_trigger
  • resource_aws_glue_user_defined_function
  • resource_aws_glue_workflow
  • resource_aws_guardduty_filter
  • resource_aws_guardduty_ipset
  • resource_aws_guardduty_organization_admin_account
  • resource_aws_guardduty_threatintelset
  • resource_aws_iam_access_key
  • resource_aws_iam_group
  • resource_aws_iam_group_membership
  • resource_aws_iam_group_policy_attachment
  • resource_aws_iam_group_policy
  • resource_aws_iam_instance_profile
  • resource_aws_iam_policy_attachment
  • resource_aws_iam_role
  • resource_aws_iam_role_policy_attachment
  • resource_aws_iam_role_policy
  • resource_aws_iam_server_certificate
  • resource_aws_iam_service_linked_role
  • resource_aws_iam_user_group_membership
  • resource_aws_iam_user_login_profile
  • resource_aws_iam_user_policy_attachment
  • resource_aws_iam_user_policy
  • resource_aws_inspector_assessment_target
  • resource_aws_instance
  • resource_aws_internet_gateway
  • resource_aws_iot_policy_attachment
  • resource_aws_iot_role_alias
  • resource_aws_iot_thing
  • resource_aws_iot_thing_principal_attachment
  • resource_aws_iot_thing_type
  • resource_aws_key_pair
  • resource_aws_kinesis_firehose_delivery_stream
  • resource_aws_kinesis_stream
  • resource_aws_kinesis_video_stream
  • resource_aws_kms_alias
  • resource_aws_kms_ciphertext
  • resource_aws_kms_grant
  • resource_aws_kms_key
  • resource_aws_lambda_alias
  • resource_aws_lambda_event_source_mapping
  • resource_aws_lambda_function
  • resource_aws_lambda_permission
  • resource_aws_lambda_provisioned_concurrency_config
  • resource_aws_launch_configuration
  • resource_aws_launch_template
  • resource_aws_lb_cookie_stickiness_policy
  • resource_aws_lb
  • resource_aws_lb_listener_certificate
  • resource_aws_lb_listener_rule
  • resource_aws_lb_target_group
  • resource_aws_lex_bot_alias
  • resource_aws_lex_bot
  • resource_aws_lex_intent
  • resource_aws_lex_slot_type
  • resource_aws_licensemanager_association
  • resource_aws_lightsail_domain
  • resource_aws_lightsail_instance
  • resource_aws_lightsail_key_pair
  • resource_aws_lightsail_static_ip_attachment
  • resource_aws_lightsail_static_ip
  • resource_aws_main_route_table_association
  • resource_aws_media_store_container
  • resource_aws_media_store_container_policy
  • resource_aws_mq_broker
  • resource_aws_mq_configuration
  • resource_aws_msk_cluster
  • resource_aws_msk_scram_secret_association
  • resource_aws_nat_gateway
  • resource_aws_neptune_cluster
  • resource_aws_neptune_cluster_instance
  • resource_aws_neptune_cluster_parameter_group
  • resource_aws_neptune_event_subscription
  • resource_aws_neptune_parameter_group
  • resource_aws_neptune_subnet_group
  • resource_aws_network_acl
  • resource_aws_network_acl_rule
  • resource_aws_network_interface_attachment
  • resource_aws_network_interface
  • resource_aws_network_interface_sg_attachment
  • resource_aws_opsworks_application
  • resource_aws_opsworks_instance
  • resource_aws_opsworks_permission
  • resource_aws_opsworks_rds_db_instance
  • resource_aws_opsworks_stack
  • resource_aws_opsworks_user_profile
  • resource_aws_organizations_account
  • resource_aws_organizations_organizational_unit
  • resource_aws_organizations_organization
  • resource_aws_organizations_policy_attachment
  • resource_aws_organizations_policy
  • resource_aws_pinpoint_adm_channel
  • resource_aws_pinpoint_app
  • resource_aws_placement_group
  • resource_aws_qldb_ledger
  • resource_aws_ram_principal_association
  • resource_aws_ram_resource_association
  • resource_aws_ram_resource_share_accepter
  • resource_aws_ram_resource_share
  • resource_aws_rds_cluster
  • resource_aws_rds_cluster_instance
  • resource_aws_rds_cluster_parameter_group
  • resource_aws_rds_global_cluster
  • resource_aws_redshift_cluster
  • resource_aws_redshift_event_subscription
  • resource_aws_redshift_parameter_group
  • resource_aws_redshift_security_group
  • resource_aws_redshift_snapshot_copy_grant
  • resource_aws_redshift_snapshot_schedule_association
  • resource_aws_redshift_subnet_group
  • resource_aws_route53_delegation_set
  • resource_aws_route53_health_check
  • resource_aws_route53_record
  • resource_aws_route53_resolver_endpoint
  • resource_aws_route53_resolver_rule_association
  • resource_aws_route53_resolver_rule
  • resource_aws_route53_zone_association
  • resource_aws_route53_zone
  • resource_aws_route
  • resource_aws_route_table_association
  • resource_aws_route_table
  • resource_aws_s3_bucket
  • resource_aws_s3_bucket_metric
  • resource_aws_s3_bucket_notification
  • resource_aws_s3_bucket_object
  • resource_aws_s3_bucket_policy
  • resource_aws_sagemaker_endpoint_configuration
  • resource_aws_sagemaker_endpoint
  • resource_aws_sagemaker_model
  • resource_aws_sagemaker_notebook_instance
  • resource_aws_security_group_rule
  • resource_aws_securityhub_action_target
  • resource_aws_securityhub_member
  • resource_aws_securityhub_product_subscription
  • resource_aws_serverlessapplicationrepository_cloudformation_stack
  • resource_aws_service_discovery_private_dns_namespace
  • resource_aws_service_discovery_public_dns_namespace
  • resource_aws_service_discovery_service
  • resource_aws_ses_active_receipt_rule_set
  • resource_aws_ses_configuration_set
  • resource_aws_ses_domain_dkim
  • resource_aws_ses_domain_identity
  • resource_aws_ses_email_identity
  • resource_aws_ses_event_destination
  • resource_aws_ses_receipt_filter
  • resource_aws_ses_receipt_rule
  • resource_aws_ses_receipt_rule_set
  • resource_aws_sfn_activity
  • resource_aws_sfn_state_machine
  • resource_aws_signer_signing_job
  • resource_aws_signer_signing_profile_permission
  • resource_aws_simpledb_domain
  • resource_aws_sns_platform_application
  • resource_aws_sns_sms_preferences
  • resource_aws_sns_topic
  • resource_aws_sns_topic_policy
  • resource_aws_sns_topic_subscription
  • resource_aws_spot_fleet_request
  • resource_aws_spot_instance_request
  • resource_aws_sqs_queue
  • resource_aws_sqs_queue_policy
  • resource_aws_ssm_association
  • resource_aws_ssm_document
  • resource_aws_ssm_maintenance_window_target
  • resource_aws_ssm_maintenance_window_task
  • resource_aws_ssm_patch_baseline
  • resource_aws_ssm_patch_group
  • resource_aws_ssm_resource_data_sync
  • resource_aws_storagegateway_cached_iscsi_volume
  • resource_aws_storagegateway_nfs_file_share
  • resource_aws_subnet
  • resource_aws_swf_domain
  • resource_aws_transfer_server
  • resource_aws_transfer_ssh_key
  • resource_aws_volume_attachment
  • resource_aws_vpc_dhcp_options_association
  • resource_aws_vpc_dhcp_options
  • resource_aws_vpc_endpoint
  • resource_aws_vpc_endpoint_route_table_association
  • resource_aws_vpc_endpoint_service_allowed_principal
  • resource_aws_vpc_endpoint_service
  • resource_aws_vpc_endpoint_subnet_association
  • resource_aws_vpc
  • resource_aws_vpc_peering_connection
  • resource_aws_vpn_connection
  • resource_aws_vpn_connection_route
  • resource_aws_vpn_gateway
  • resource_aws_waf_byte_match_set
  • resource_aws_waf_geo_match_set
  • resource_aws_waf_ipset
  • resource_aws_waf_rate_based_rule
  • resource_aws_waf_regex_match_set
  • resource_aws_waf_regex_pattern_set
  • resource_aws_wafregional_ipset
  • resource_aws_wafregional_rule
  • resource_aws_wafregional_rule_group
  • resource_aws_wafregional_web_acl_association
  • resource_aws_wafregional_web_acl
  • resource_aws_wafregional_xss_match_set
  • resource_aws_waf_rule
  • resource_aws_waf_rule_group
  • resource_aws_waf_size_constraint_set
  • resource_aws_waf_sql_injection_match_set
  • resource_aws_wafv2_ip_set
  • resource_aws_wafv2_regex_pattern_set
  • resource_aws_wafv2_rule_group
  • resource_aws_wafv2_web_acl_association
  • resource_aws_wafv2_web_acl
  • resource_aws_waf_web_acl
  • resource_aws_waf_xss_match_set
  • resource_aws_worklink_website_certificate_authority_association
  • resource_aws_workspaces_directory
  • resource_aws_workspaces_ip_group
  • resource_aws_workspaces_workspace
  • resource_aws_xray_sampling_rule

@bflad
Copy link
Contributor Author

bflad commented Dec 15, 2020

@mattburgess before you get too far, we should work through your existing pull requests (ensuring they all still pass acceptance testing) and discuss the intended fixes here.

@bflad
Copy link
Contributor Author

bflad commented Dec 17, 2020

Here's some of the missing Contributing Guide documentation: #16794 👍

@github-actions
Copy link

github-actions bot commented Dec 7, 2022

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

@github-actions github-actions bot added the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Dec 7, 2022
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 7, 2023
@github-actions
Copy link

github-actions bot commented Feb 7, 2023

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
linter Pertains to changes to or issues with the various linters. stale Old or inactive issues managed by automation, if no further action taken these will get closed. technical-debt Addresses areas of the codebase that need refactoring or redesign. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

No branches or pull requests

2 participants