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

Authdriver #236

Merged
merged 16 commits into from
May 23, 2017
Merged

Conversation

samvarankashyap
Copy link
Collaborator

@samvarankashyap samvarankashyap commented May 15, 2017

Summary:

Designed according to PR #76 discussion
Updated schema v4
Updated playbooks

example topology :

---
    topology_name: "ex_os_server"
    resource_groups:
      - 
        resource_group_name: "testgroup1"
        res_group_type: "openstack"
        res_defs:
          - 
            res_name: "sktestinstance"
            flavor: "m1.small"
            res_type: "os_server"
            image: "rhel-6.5_jeos"
            count: 3
            keypair: "ci-factory"
            networks:
              - "atomic-e2e-jenkins-test"
              - "atomic-e2e-jenkins-test2"
            fip_pool: "10.8.172.0/22"
        credentials:
          name: "atomic_jenkins"
          profile: "default"  # optional if not mentioned it defaults to default
          auth_type: "file" # optional defaults to "file" no other implementations yet
    resource_group_vars:
      - 
        resource_group_name : "testgroup1"
        test_var1: "test_var1 msg is grp1 hello "
        test_var2: "test_var2 msg is grp1 hello "
        test_var3: "test_var3 msg is grp1 hello "

cli usage:

linchpin rise --creds /path/to/creds

or

export LP_CREDS='/path/to/creds'
linchpin rise

Issue fixes:

fixes #76

Note:
** should be merged after #218 **
** Needs Rebase **
** Needs testing (for gcloud) **

@@ -60,12 +60,15 @@ def get_command(self, ctx, name):
@click.option('-w', '--workspace', type=click.Path(), envvar='WORKSPACE',
help='Use the specified workspace if the familiar Jenkins $WORKSPACE environment variable '
'is not set')
@click.option('-c', '--creds', type=click.Path(), envvar='LP_CREDS',
Copy link
Contributor

Choose a reason for hiding this comment

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

the -c option is already used for config. Just use --creds-path option here.

@click.option('-v', '--verbose', is_flag=True, default=False,
help='Enable verbose output')
@click.option('--version', is_flag=True,
help='Prints the version and exits')
@pass_context
def runcli(ctx, config, workspace, verbose, version):
def runcli(ctx, config, workspace, creds, verbose, version):
Copy link
Contributor

Choose a reason for hiding this comment

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

When adding new args, please add them at the end as a keyword argument. This is especially important for api functionality. In this case, it's unlikely to be a kwarg, but creds_path should be at the end.

@@ -36,6 +36,7 @@ schemas_folder = schemas

resources_folder = resources
inventories_folder = inventories
credentials_folder = credentials
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary? I don't see where it's currently used.

…ll string

Current built in default jinja filter helps the user to omit/provide default to keys
in the ansible module. However , it doesnt provide a way for user to conditionally
provide a default value to ie., based on the value provided by the output of previous
filter . In order to facilitate the same provide_default filter is being written.
auth_driver module fetches and parses file returns to credentials in auth_var
variable making creds accessible inside ansible playbooks.
@samvarankashyap
Copy link
Collaborator Author

PR is broken after rebase, will be fixing it soon .

@samvarankashyap
Copy link
Collaborator Author

linchpin up on openstack

$ linchpin --creds-path ./credentials/ up openstack
target: openstack, action: up
PLAY [schema check and Pre Provisioning Activities on topology_file] ***********
...
TASK [inventory_gen : Updating inventory_file with the absolute path] **********
ok: [localhost]

TASK [inventory_gen : Generate Generic Inventory] ******************************
ok: [localhost]

PLAY RECAP *********************************************************************
localhost                  : ok=33   changed=3    unreachable=0    failed=0   

linchpin destroy on openstack

$ linchpin --creds-path ./credentials/ destroy openstack
target: openstack, action: destroy


PLAY [schema check and Pre Provisioning Activities on topology_file] ***********

TASK [common : use linchpin_config if provided] ********************************
skipping: [localhost]
...
...
PLAY RECAP *********************************************************************
localhost                  : ok=27   changed=2    unreachable=0    failed=0   

@@ -188,6 +188,8 @@ def run_playbook(self, pinfile, targets='all', playbook='up'):
if self.ctx.cfgs.get('ansible'):
ansible_console = ast.literal_eval(self.ctx.cfgs['ansible'].get('console', 'False'))

self.ctx.evars['creds_path'] = self.ctx.creds_path
Copy link
Contributor

@herlo herlo May 22, 2017

Choose a reason for hiding this comment

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

Use self.set_evar here. Failed CI due to the value not being there. set_evar makes sure to create the structure for you if it isn't there.

@@ -188,7 +188,7 @@ def run_playbook(self, pinfile, targets='all', playbook='up'):
if self.ctx.cfgs.get('ansible'):
ansible_console = ast.literal_eval(self.ctx.cfgs['ansible'].get('console', 'False'))

self.ctx.evars['creds_path'] = self.ctx.creds_path
self.set_evar('creds_path', self.ctx.creds_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not help. self.ctx.creds_path is being set into the same place. Why is this necessary?

@samvarankashyap
Copy link
Collaborator Author

linchpin up aws # with env vars

linchpin up aws
target: aws, action: up


PLAY [schema check and Pre Provisioning Activities on topology_file] ***********

TASK [common : use linchpin_config if provided] ********************************
skipping: [localhost]
...
TASK [inventory_gen : Generate Generic Inventory] ******************************
changed: [localhost]

PLAY RECAP *********************************************************************
localhost                  : ok=33   changed=3    unreachable=0    failed=0   

linchpin destroy aws

$ linchpin destroy aws
target: aws, action: destroy


PLAY [schema check and Pre Provisioning Activities on topology_file] ***********

TASK [common : use linchpin_config if provided] ********************************
skipping: [localhost]

TASK [common : set lp_path] ****************************************************
skipping: [localhost]
...
TASK [inventory_gen : Generate Generic Inventory] ******************************
skipping: [localhost]
PLAY RECAP *********************************************************************
localhost                  : ok=28   changed=2    unreachable=0    failed=0 

@samvarankashyap
Copy link
Collaborator Author

samvarankashyap commented May 23, 2017

linchpin up aws # with --creds-path

$ linchpin --creds-path ../workspace/creds/ up aws
target: aws, action: up
PLAY [schema check and Pre Provisioning Activities on topology_file] ***********
TASK [common : use linchpin_config if provided] ********************************
skipping: [localhost]
...
TASK [inventory_gen : Generate Generic Inventory] ******************************
changed: [localhost]

PLAY RECAP *********************************************************************
localhost                  : ok=33   changed=4    unreachable=0    failed=0   

linchpin destroy # with --creds-path

$ linchpin --creds-path ../workspace/creds/ destroy aws
target: aws, action: destroy


PLAY [schema check and Pre Provisioning Activities on topology_file] ***********

TASK [common : use linchpin_config if provided] ********************************
skipping: [localhost]
...
...
TASK [inventory_gen : Generate Generic Inventory] ******************************
skipping: [localhost]

PLAY RECAP *********************************************************************
localhost                  : ok=28   changed=3    unreachable=0    failed=0   

@samvarankashyap
Copy link
Collaborator Author

linchpin up gcloud # with creds-path

$ linchpin --creds-path /home/srallaba/workspace/creds/ up gcloud
target: gcloud, action: up
PLAY [schema check and Pre Provisioning Activities on topology_file] ***********
TASK [common : use linchpin_config if provided] ********************************
skipping: [localhost]
...
...

TASK [inventory_gen : Generate Generic Inventory] ******************************
changed: [localhost]
PLAY RECAP *********************************************************************
localhost                  : ok=32   changed=4    unreachable=0    failed=0   

linchpin destroy gcloud # with creds-path

linchpin --creds-path /home/srallaba/workspace/creds/ destroy gcloud
target: gcloud, action: destroy
PLAY [schema check and Pre Provisioning Activities on topology_file] ***********
TASK [common : use linchpin_config if provided] ********************************
skipping: [localhost]
...
...
TASK [inventory_gen : Generate Generic Inventory] ******************************
skipping: [localhost]

PLAY RECAP *********************************************************************
localhost                  : ok=26   changed=2    unreachable=0    failed=0   

@samvarankashyap
Copy link
Collaborator Author

Note: All the above tests are performed on schema_v4 topologies. As credentials attribute is introduced in schema_v4 only .
example workspace for testing including the topologies can be found in following link.
linchtest

@herlo
Copy link
Contributor

herlo commented May 23, 2017

What is the likelihood of removing schema_v3 with this update? Probably shouldn't be maintaining two schemas as that's not a viable option.

@samvarankashyap
Copy link
Collaborator Author

samvarankashyap commented May 23, 2017

@herlo I also feel we should be using schema_v4 only for auth_driver because it makes no sense in supporting both the schemas from 1.0 release. However , the only consequence is to update all the examples and documentation for openstack , gcloud and aws topologies.

@herlo
Copy link
Contributor

herlo commented May 23, 2017

@samvarankashyap since we agree on this point, please create an issue to remove schema_v3 as part of a 1.1.0 release moving forward.

Copy link
Contributor

@herlo herlo left a comment

Choose a reason for hiding this comment

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

LGTM

@samvarankashyap
Copy link
Collaborator Author

@herlo: ref #243

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(POC) (8) Manage credentials according to the cloud provider
2 participants