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

Add Ability to customize the Secret Variable name in the generated YAML #969

Closed
dansiegel opened this issue Jul 3, 2022 · 5 comments
Closed

Comments

@dansiegel
Copy link
Contributor

Description

There may be times where it is prudent to reuse certain targets that make user of secret parameters. One such case could be for building an iOS App. This app may need to be built for a Dev/QA Environment where only a Development Code Signing Certificate is used along with a Development Provisioning Profile. Other jobs may need to reuse the same Target to build the iOS app only for release to the store with a different variable for the Production Cert/Profile.

As also shown in #968 the WorkflowJob from the Nuke.Maui repo implements this so that you can provide either just the name or the name of the parameter with the name of the secret to reference. Doing this allows us to reuse the same Targets while customizing which secrets we want to use for that job.

Usage Example

[WorkflowJob(
    Name = "publish-internal",
    Needs = new[] { "compile-lib", "android-build", "ios-build" },
    DownloadArtifacts = new[] { "nuget" },
    CheckoutRepository = false,
    ImportSecrets = new[]
    {
        nameof(IPublishInternal.InHouseNugetFeed),
        nameof(IPublishInternal.InHouseApiKey),
        $"{nameof(ICodeSignNuget.CodeSignCertificate)}=CODESIGNCERTIFICATE",
        $"{nameof(ICodeSignNuget.CodeSignClientId)}=CODESIGNCLIENTID",
        $"{nameof(ICodeSignNuget.CodeSignClientSecret)}=CODESIGNCLIENTSECRET",
        $"{nameof(ICodeSignNuget.CodeSignKeyVault)}=CODESIGNKEYVAULT",
        $"{nameof(ICodeSignNuget.CodeSignTenantId)}=CODESIGNTENANTID"
    },
    InvokedTargets = new[] { nameof(IPublishInternal.PublishNuGet) })]

Alternative

No response

@matkoch
Copy link
Member

matkoch commented Jul 3, 2022

I wouldn't do this. It completely breaks the benefit of having an integrated way, where you can use nameof to keep everything in sync and predictable.

Why not have two properties ProdSecret and TestSecret and make a fallback via ?? ? You could also introduce an expression bodied property to keep the target more concise.

@dansiegel
Copy link
Contributor Author

I mean if you take the actual sample above, I have organization wide variables that have specific naming which doesn't match how Nuke would auto generate the Secret name.

In the case of something like an iOS build this significantly over complicates things. With something like what I have above you have the benefit of being able to control the name of the secret at the Job level without having to touch any code which is ultimately as it should be because the job simply needs to know what certificate am I using what profile am I using... it should not need to know about environments that the Target may be reused across.

@matkoch
Copy link
Member

matkoch commented Jul 3, 2022

The IEnumerable<(string Key, string Value)> GetImports() method, which is used to generate the imports, is virtual, so if this really is required, then there's a chance to override the behavior.

Providing this feature out of the box, however, increases the complexity of other related features. For instance #959, would allow setting secrets from the CLI, but if environment keys were to be set in the attribute, that information would need to go into build.schema.json. And if you assume people use different environment keys for different pipelines, it'd be even more complex.

@dansiegel
Copy link
Contributor Author

Sorry I just realized I only showed the C# side here... We're not talking about trying to change the parameter names used by Nuke... only providing a way that someone can optionally override the default name that is used when generating the yaml for GitHubActions or whatever CI platform. You'll notice in this example that where I just passed in the parameter name it continues to work as expected... where I have the equals syntax it overrides the default name of the Secret in the generated YAML... nothing actually changes about the way Nuke handles properties here.

[WorkflowJob(
    Name = MacCatalystBuild,
    Image = HostedAgent.Mac,
    InvokedTargets = new[] { nameof(IHazMacCatalystBuild.CompileMacCatalyst) },
    ImportSecrets = new[]
    {
        nameof(IHazAppleCertificate.P12B64),
        nameof(IHazAppleCertificate.P12Password),
        nameof(IRestoreAppleProvisioningProfile.AppleIssuerId),
        nameof(IRestoreAppleProvisioningProfile.AppleKeyId),
        nameof(IRestoreAppleProvisioningProfile.AppleAuthKeyP8),
        $"{nameof(IRestoreAppleProvisioningProfile.AppleProfileId)}=MACCATALYST_PROVISIONING_PROFILE"
    })]
- name: Run './build.sh CompileMacCatalyst'
  run: ./build.sh CompileMacCatalyst
  env:
    P12B64: ${{ secrets.P12B64 }}
    P12Password: ${{ secrets.P12PASSWORD }}
    AppleIssuerId: ${{ secrets.APPLE_ISSUER_ID }}
    AppleKeyId: ${{ secrets.APPLE_KEY_ID }}
    AppleAuthKeyP8: ${{ secrets.APPLE_AUTH_KEY_P8 }}
    AppleProfileId: ${{ secrets.MACCATALYST_PROVISIONING_PROFILE }}

@matkoch
Copy link
Member

matkoch commented Nov 29, 2022

Re-read my last comment, that's still the culprit.

@matkoch matkoch closed this as not planned Won't fix, can't repro, duplicate, stale Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants