theopenlab / openlab

Used for general work tracking, centralized reporting, easier third-party integration for metrics and data, and more.
Apache License 2.0
5 stars 1 forks source link

Terraform-provider-openstack: support admin tests #365

Closed ozerovandrei closed 3 years ago

ozerovandrei commented 5 years ago

Hello.

Could you please enable admin privileges for the OpenStack user that is authenticated in acceptance tests for https://github.com/terraform-providers/terraform-provider-openstack.

We have some acceptance tests that require admin privileges in OpenStack and they got skipped during pull requests runs.

We don't check any environment variable for those tests (like it's done in lbaas, designate and others) and instead of that we're just checking if username equals to admin.

I hope that it won't be a difficult task given that Gophercloud already uses admin user in its acceptance tests.

huangtianhua commented 5 years ago

Sorry, I am not sure what do you mean? AFAIK, first there are tests of terraform require admin privileges like keystone actions(create user...) and they are supported for a long time; second, the environment variable OS_USERNAME we set 'admin' already. So could you describe this with more details, or maybe there is some tests failed in our jobs? Please give the link then we would have a look into it. Thanks.

ozerovandrei commented 5 years ago

It seems that we don't have OS_USERNAME set to admin since all Identity tests are skipped https://logs.openlabtesting.org/logs/99/899/dd3019cfd17eb0d44c22475f8af3e9ddeebde3da/check/terraform-provider-openstack-acceptance-test/692ccec/:

2019-10-13 12:54:49.047347 | ubuntu-xenial | === RUN   TestAccIdentityV3Endpoint_basic
2019-10-13 12:54:49.047443 | ubuntu-xenial | --- SKIP: TestAccIdentityV3Endpoint_basic (0.00s)
2019-10-13 12:54:49.047554 | ubuntu-xenial |     provider_test.go:201: Skipping test because it requires the admin user
2019-10-13 12:54:49.047640 | ubuntu-xenial | === RUN   TestAccIdentityV3Project_basic
2019-10-13 12:54:49.047773 | ubuntu-xenial | --- SKIP: TestAccIdentityV3Project_basic (0.00s)
2019-10-13 12:54:49.047878 | ubuntu-xenial |     provider_test.go:201: Skipping test because it requires the admin user
2019-10-13 12:54:49.047964 | ubuntu-xenial | === RUN   TestAccIdentityV3RoleAssignment_basic
2019-10-13 12:54:49.048056 | ubuntu-xenial | --- SKIP: TestAccIdentityV3RoleAssignment_basic (0.00s)
2019-10-13 12:54:49.048198 | ubuntu-xenial |     provider_test.go:201: Skipping test because it requires the admin user
2019-10-13 12:54:49.048262 | ubuntu-xenial | === RUN   TestAccIdentityV3Role_basic
2019-10-13 12:54:49.048313 | ubuntu-xenial | --- SKIP: TestAccIdentityV3Role_basic (0.00s)
2019-10-13 12:54:49.048412 | ubuntu-xenial |     provider_test.go:201: Skipping test because it requires the admin user
2019-10-13 12:54:49.048470 | ubuntu-xenial | === RUN   TestAccIdentityV3Service_basic
2019-10-13 12:54:49.048523 | ubuntu-xenial | --- SKIP: TestAccIdentityV3Service_basic (0.00s)
2019-10-13 12:54:49.048592 | ubuntu-xenial |     provider_test.go:201: Skipping test because it requires the admin user
2019-10-13 12:54:49.048678 | ubuntu-xenial | === RUN   TestAccIdentityV3User_basic
2019-10-13 12:54:49.048784 | ubuntu-xenial | --- SKIP: TestAccIdentityV3User_basic (0.00s)
2019-10-13 12:54:49.048862 | ubuntu-xenial |     provider_test.go:201: Skipping test because it requires the admin user

This message is from that function https://github.com/terraform-providers/terraform-provider-openstack/blob/master/openstack/provider_test.go#L198:

func testAccPreCheckAdminOnly(t *testing.T) {
    v := os.Getenv("OS_USERNAME")
    if v != "admin" {
        t.Skip("Skipping test because it requires the admin user")
    }
}
kayrus commented 5 years ago

there is another PR, which requires admin username to test: https://github.com/terraform-providers/terraform-provider-openstack/pull/628

jtopjian commented 4 years ago

I think this is due to this line here: https://github.com/theopenlab/openlab-zuul-jobs/blob/e02e4bc571b9ac30fb29b9b71ffe53f2b0c2cfa9/playbooks/terraform-provider-openstack-acceptance-test/run.yaml#L38

In the beginning, it was intentional to run the Terraform tests as the demo user instead of admin because some of the Terraform tests would fail as admin. If the above line is removed, that should allow the tests to run as admin, but some tests might need updated.

kayrus commented 4 years ago

Another example:

2020/01/15 13:40:16 [DEBUG] OpenStack Request URL: GET http://192.168.0.12/image/v2/tasks
2020/01/15 13:40:16 [DEBUG] OpenStack Request Headers:
Accept: application/json
Cache-Control: no-cache
User-Agent: HashiCorp Terraform/0.12.7-sdk (+https://www.terraform.io) Terraform Plugin SDK/1.4.1 gophercloud/2.0.0 
X-Auth-Token: ***
2020/01/15 13:40:16 [DEBUG] OpenStack Response Code: 403
2020/01/15 13:40:16 [DEBUG] OpenStack Response Headers: 
Content-Length: 115
Content-Type: application/json
Date: Wed, 15 Jan 2020 13:40:16 GMT
Server: Apache/2.4.29 (Ubuntu)
X-Openstack-Request-Id: req-36246e25-466a-4593-b233-730a1ec60ba1
2020/01/15 13:40:16 [DEBUG] OpenStack Response Body: {
  "code": "403 Forbidden",
  "message": "Access was denied to this resource.\u003cbr /\u003e\u003cbr /\u003e\n\n\n",
  "title": "Forbidden"
}

@jtopjian @ozerovandrei I suppose that we need to introduce an extra job, which will run acceptance tests with admin privileges. This will allow to test both user and admin cases.

jtopjian commented 4 years ago

I suppose that we need to introduce an extra job, which will run acceptance tests with admin privileges. This will allow to test both user and admin cases.

The reason the tests don't run as admin right now is because there are certain tests which fail as admin. An example of this is the compute instance tests which validate "implicit" networking - ensuring that an instance will be launched on a user's only network when no network block is specified. The admin user usually has access to two networks (an internal network for the admin tenant and the external network used for floating IPs).

Both switching the current test suite to run as admin or creating a separate job for the admin user will both fail in the same way because of tests like these. I can see two options:

  1. Disable tests like the one described above altogether.
  2. Add conditionals for tests which which should not be run as an admin user and then add a second job that runs as admin.
kayrus commented 4 years ago
  1. Add conditionals for tests which which should not be run as an admin user and then add a second job that runs as admin.

I vote for this option.