[FEAT] Add github_repository_pages Resource and Data Source

[deiga]

Resolves #3167
Resolves #2671
Resolves #3142
Resolves #1045
Resolves #2653
Resolves #2335

---

Before the change?

• A repository's Pages resource was only manageable through github_repository

After the change?

• There is now a github_repository_pages resource to manage the pages
• There is now a github_repository_pages data source to fetch information about the pages
• The pages fields are now deprecated in github_repository Resource and Data Source
• It's possible to configure the public field of github_repository_pages
• It's possible to configure the https_enforced field of github_repository_pages

Pull request checklist

• Schema migrations have been created if needed (example)
• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

• Yes
• No

---

[github-actions]

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

[stevehipwell]

This is looking really good.

[healiha]

Would it be possible to add visibility to github_repository_pages?

The documentation doesn't seem to cover this, but after testing, a PUT request to https://api.github.com/repos/<org>/<repo>/pages with {"public": true/false} works.

[deiga]

@healiha I'll check that out

[stevehipwell]

LGTM

[stevehipwell]

@deiga could you please rebase this PR?

[stevehipwell]

Suggested change

	// Determine if we need to update the page with CNAME or public flag
	shouldUpdatePage := false
	update := &github.PagesUpdate{}
	cname, cnameExists := d.GetOk("cname")
	if cnameExists && cname.(string) != "" {
	shouldUpdatePage = true
	update.CNAME = github.Ptr(cname.(string))
	}
	public, publicExists := d.GetOkExists("public") // nolint:staticcheck // SA1019: There is no better alternative for checking if boolean value is set
	if publicExists && public != nil {
	shouldUpdatePage = true
	update.Public = github.Ptr(public.(bool))
	} else {
	if err := d.Set("public", pages.GetPublic()); err != nil {
	return diag.FromErr(err)
	}
	}
	httpsEnforced, httpsEnforcedExists := d.GetOkExists("https_enforced") // nolint:staticcheck // SA1019: There is no better alternative for checking if boolean value is set
	if httpsEnforcedExists && httpsEnforced != nil {
	shouldUpdatePage = true
	update.HTTPSEnforced = github.Ptr(httpsEnforced.(bool))
	} else {
	if err := d.Set("https_enforced", pages.GetHTTPSEnforced()); err != nil {
	return diag.FromErr(err)
	}
	}
	if shouldUpdatePage {
	_, err = client.Repositories.UpdatePages(ctx, owner, repoName, update)
	if err != nil {
	return diag.FromErr(err)
	}
	}
	// Check if we have values set that can't be configured as part of the create logic
	cname, cnameOK := d.GetOk("cname")
	public := d.get("public").(bool)
	httpsEnforced := d.Get("https_enforced").(bool)
	if cnameOK || public || httpsEnforced {
	update := &github.PagesUpdate{}
	if cnameOK {
	update.CNAME = github.Ptr(cname.(string))
	}
	if public {
	update.Public = github.Ptr(public)
	}
	if httpsEnforced {
	update.HTTPSEnforced = github.Ptr(httpsEnforced)
	}
	_, err = client.Repositories.UpdatePages(ctx, owner, repoName, update)
	if err != nil {
	return diag.FromErr(err)
	}
	}

The logic for this can be significantly simplified as we only care about true values and this code is only run for the create.

[deiga]

Thanks! We do care are about the false value for public too, since a non-EMU GHEC can set either.
But I'll adopt your suggestion with that modification!

[stevehipwell]

What I mean is that the unset value matches the default value. We only need to use the hacky method if we care about differentiating between false and unset.

In update, I'd expect the value to be set as we usually do rather than checking for a diff. Unless there is a specific reason not to?

[deiga]

I restructured it, but had to re-add the d.Set calls as otherwise the computed values won't be set

[stevehipwell]

We generally don't use this pattern, I'd expect to see d.get() & d.GetOk() here.

[deiga]

I'm not sure I understand what you mean. d.HasChange has been used ~36 times in the codebase.
d.HasChange() tells us if there is a value change in the plan, whereas d.Get() and d.GetOk() would cause us to send fields even if they are not changed

[stevehipwell]

I don't think 36 usages, some of which are definitely in diff functions, comes close to the occurrences of the get functions. The only reason you're in the update function is that there are changes to apply, so building the struct using the get functions (d.Get() for bools & d.GetOk() for strings where that aren't always set) is going to be the most efficient and readable pattern.

[deiga]

Since the API supports partial updates, I don't fully understand why we wouldn't want to utilize that.

For example these Optional & Computed fields are things that I wouldn't want to send unless they are actually changed in the plan and have been modified by the user.
Though I guess d.GetOk() isn't able to determine the latter once they have been set by Create or Read?

Could you elaborate the issue you're seeing with using d.HasChange()?